diff --git a/classes/condition.php b/classes/condition.php
index 5a65906..9aa3131 100644
--- a/classes/condition.php
+++ b/classes/condition.php
@@ -193,15 +193,11 @@ public static function get_exception(int $courseid, int $userid): bool {
public static function set_exception(int $courseid, int $userid, bool $skipauth): void {
global $DB;
// Insert or update exception record.
- $data = new stdClass();
- $data->courseid = $courseid;
- $data->userid = $userid;
- $data->skipauth = $skipauth;
- if ($id = $DB->get_field('availability_shibboleth2fa_e', 'id', ['courseid' => $courseid, 'userid' => $userid])) {
- $data->id = $id;
- $DB->update_record('availability_shibboleth2fa_e', $data);
+ $data = ['courseid' => $courseid, 'userid' => $userid];
+ if ($id = $DB->get_field('availability_shibboleth2fa_e', 'id', $data)) {
+ $DB->update_record('availability_shibboleth2fa_e', $data + ['skipauth' => $skipauth, 'id' => $id]);
} else {
- $DB->insert_record('availability_shibboleth2fa_e', $data);
+ $DB->insert_record('availability_shibboleth2fa_e', $data + ['skipauth' => $skipauth]);
}
// Invalidate exception cache.
if ($userid == self::$exceptioncacheuser) {
diff --git a/classes/exception_current_user_selector.php b/classes/exception_current_user_selector.php
deleted file mode 100644
index 4b394e7..0000000
--- a/classes/exception_current_user_selector.php
+++ /dev/null
@@ -1,109 +0,0 @@
-.
-
-/**
- * Definition of the {@see exception_current_user_selector} class.
- *
- * @package availability_shibboleth2fa
- * @copyright 2021 Lars Bonczek, innoCampus, TU Berlin
- * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
- */
-
-namespace availability_shibboleth2fa;
-
-use coding_exception;
-use dml_exception;
-use user_selector_base;
-
-defined('MOODLE_INTERNAL') || die();
-global $CFG;
-require_once("$CFG->dirroot/user/selector/lib.php");
-
-
-/**
- * TODO: Reduce code duplication with {@see exception_potential_user_selector}
- */
-class exception_current_user_selector extends user_selector_base {
- /** @var int ID of the course that the exceptions apply to */
- protected int $courseid;
-
- /**
- * {@inheritDoc}
- *
- * @param string $name The control name/id for use in the HTML.
- * @param array $options Other options needed to construct this selector. Must contain the `courseid`.
- */
- public function __construct($name, $options) {
- $this->courseid = $options['courseid'];
- parent::__construct($name, $options);
- }
-
- /**
- * Returns users for whom exceptions were defined in the associated course.
- * {@inheritDoc}
- *
- * @param string $search the search string.
- * @return array An array of arrays of users. The array keys of the outer array should be the string names of optgroups.
- * The keys of the inner arrays should be userids, and the values should be user objects containing at least
- * the list of fields returned by the method required_fields_sql(). If a user object has a ->disabled property
- * that is true, then that option will be displayed greyed out, and will not be returned by get_selected_users.
- * @throws coding_exception
- * @throws dml_exception
- */
- public function find_users($search): array {
- global $DB;
- // By default, wherecondition retrieves all users except the deleted, not confirmed and guest.
- [$wherecondition, $params] = $this->search_sql($search, 'u');
- $params['courseid'] = $this->courseid;
- $fields = "SELECT {$this->required_fields_sql('u')}";
- $countfields = 'SELECT COUNT(1)';
- $sql = " FROM {user} u
- JOIN {user_enrolments} AS ue ON ue.userid = u.id
- JOIN {enrol} AS en ON ue.enrolid = en.id
- JOIN {course} AS course ON en.courseid = course.id
- JOIN {availability_shibboleth2fa_e} e ON (e.userid = u.id AND e.courseid = course.id AND e.skipauth = 1)
- WHERE $wherecondition
- AND course.id = :courseid";
- [$sort, $sortparams] = users_order_by_sql('u', $search, $this->accesscontext);
- $order = " ORDER BY $sort";
- if (!$this->is_validating()) {
- $potentialmemberscount = $DB->count_records_sql($countfields . $sql, $params);
- if ($potentialmemberscount > $this->maxusersperpage) {
- return $this->too_many_results($search, $potentialmemberscount);
- }
- }
- $availableusers = $DB->get_records_sql($fields . $sql . $order, array_merge($params, $sortparams));
- if (empty($availableusers)) {
- return [];
- }
- if ($search) {
- $groupname = get_string('users_with_exception_matching', 'availability_shibboleth2fa', $search);
- } else {
- $groupname = get_string('users_with_exception', 'availability_shibboleth2fa');
- }
- return [$groupname => $availableusers];
- }
-
- /**
- * {@inheritDoc}
- */
- protected function get_options(): array {
- $options = parent::get_options();
- $options['courseid'] = $this->courseid;
- $options['file'] = '/availability/condition/shibboleth2fa/classes/exception_current_user_selector.php';
- return $options;
- }
-}
diff --git a/classes/exception_potential_user_selector.php b/classes/exception_potential_user_selector.php
deleted file mode 100644
index 5a982f9..0000000
--- a/classes/exception_potential_user_selector.php
+++ /dev/null
@@ -1,111 +0,0 @@
-.
-
-/**
- * Definition of the {@see exception_potential_user_selector} class.
- *
- * @package availability_shibboleth2fa
- * @copyright 2021 Lars Bonczek, innoCampus, TU Berlin
- * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
- */
-
-namespace availability_shibboleth2fa;
-
-
-use coding_exception;
-use dml_exception;
-use user_selector_base;
-
-defined('MOODLE_INTERNAL') || die();
-global $CFG;
-require_once("$CFG->dirroot/user/selector/lib.php");
-
-
-/**
- * TODO: Reduce code duplication with {@see exception_current_user_selector}
- */
-class exception_potential_user_selector extends user_selector_base {
- /** @var int ID of the course that the exceptions apply to */
- protected int $courseid;
-
- /**
- * {@inheritDoc}
- *
- * @param string $name The control name/id for use in the HTML.
- * @param array $options Other options needed to construct this selector. Must contain the `courseid`.
- */
- public function __construct($name, $options) {
- $this->courseid = $options['courseid'];
- parent::__construct($name, $options);
- }
-
- /**
- * Returns candidate users for whom exceptions can be set in the associated course.
- * {@inheritDoc}
- *
- * @param string $search the search string.
- * @return array An array of arrays of users. The array keys of the outer array should be the string names of optgroups.
- * The keys of the inner arrays should be userids, and the values should be user objects containing at least
- * the list of fields returned by the method required_fields_sql(). If a user object has a ->disabled property
- * that is true, then that option will be displayed greyed out, and will not be returned by get_selected_users.
- * @throws coding_exception
- * @throws dml_exception
- */
- public function find_users($search): array {
- global $DB;
- // By default, wherecondition retrieves all users except the deleted, not confirmed and guest.
- [$wherecondition, $params] = $this->search_sql($search, 'u');
- $params['courseid'] = $this->courseid;
- $fields = "SELECT {$this->required_fields_sql('u')}";
- $countfields = 'SELECT COUNT(1)';
- $sql = " FROM {user} u
- JOIN {user_enrolments} AS ue ON ue.userid = u.id
- JOIN {enrol} AS en ON ue.enrolid = en.id
- JOIN {course} AS course ON en.courseid = course.id
- LEFT JOIN {availability_shibboleth2fa_e} e ON (e.userid = u.id AND e.courseid = course.id AND e.skipauth = 1)
- WHERE $wherecondition
- AND e.id IS NULL
- AND course.id = :courseid";
- [$sort, $sortparams] = users_order_by_sql('u', $search, $this->accesscontext);
- $order = " ORDER BY $sort";
- if (!$this->is_validating()) {
- $potentialmemberscount = $DB->count_records_sql($countfields . $sql, $params);
- if ($potentialmemberscount > $this->maxusersperpage) {
- return $this->too_many_results($search, $potentialmemberscount);
- }
- }
- $availableusers = $DB->get_records_sql($fields . $sql . $order, array_merge($params, $sortparams));
- if (empty($availableusers)) {
- return [];
- }
- if ($search) {
- $groupname = get_string('users_without_exception_matching', 'availability_shibboleth2fa', $search);
- } else {
- $groupname = get_string('users_without_exception', 'availability_shibboleth2fa');
- }
- return [$groupname => $availableusers];
- }
-
- /**
- * {@inheritDoc}
- */
- protected function get_options(): array {
- $options = parent::get_options();
- $options['courseid'] = $this->courseid;
- $options['file'] = '/availability/condition/shibboleth2fa/classes/exception_potential_user_selector.php';
- return $options;
- }
-}
diff --git a/classes/user_exception_selector.php b/classes/user_exception_selector.php
new file mode 100644
index 0000000..a42155d
--- /dev/null
+++ b/classes/user_exception_selector.php
@@ -0,0 +1,205 @@
+.
+
+/**
+ * Definition of the {@see user_exception_selector} class.
+ *
+ * @package availability_shibboleth2fa
+ * @copyright 2024 innoCampus, TU Berlin
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+namespace availability_shibboleth2fa;
+
+use coding_exception;
+use context_course;
+use dml_exception;
+use user_selector_base;
+
+defined('MOODLE_INTERNAL') || die();
+global $CFG;
+require_once("$CFG->dirroot/user/selector/lib.php");
+
+/**
+ * Selector for exceptions to the 2FA requirement on a per user-and-course basis.
+ *
+ * @package availability_shibboleth2fa
+ * @copyright 2024 innoCampus, TU Berlin
+ * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class user_exception_selector extends user_selector_base {
+ /** @var int ID of the course that the exceptions apply to */
+ protected int $courseid;
+
+ /**
+ * @var bool Whether the instance is for selecting among existing user exceptions.
+ * If `true`, the selector will allow finding users, who are currently exempt from the 2FA requirement in a course.
+ * If `false`, it will allow finding users, who are currently not exempt from the 2FA requirement in a course.
+ */
+ protected bool $skipauth;
+
+ /**
+ * Constructs and returns a new instance for the specified course.
+ *
+ * This method is a wrapper around the regular constructor for convenience and type safety.
+ *
+ * @param int $courseid ID of the course that the exceptions apply to
+ * @param bool $skipauth Whether the selector is intended to find users that are already exempt from the 2FA requirement in the
+ * course (`true` value) or those that are not (`false` value).
+ * @return self Selector instance for finding users and modifying exceptions in the course.
+ */
+ public static function instance(int $courseid, bool $skipauth): self {
+ return new self(
+ name: $skipauth ? 'removeselect' : 'addselect',
+ options: [
+ 'courseid' => $courseid,
+ 'skipauth' => $skipauth,
+ 'accesscontext' => context_course::instance($courseid),
+ ],
+ );
+ }
+
+ /**
+ * {@inheritDoc}
+ *
+ * @param string $name The control name/id for use in the HTML.
+ * @param array $options Other options needed to construct this selector. Must contain the course ID as an integer under the
+ * key `courseid`, as well as a boolean value under the `skipauth` key to indicate whether the selector is
+ * intended to find users that are already exempt from the 2FA requirement in the course (`true` value) or
+ * those that are not (`false` value).
+ */
+ public function __construct($name, $options) {
+ $this->courseid = $options['courseid'];
+ $this->skipauth = $options['skipauth'];
+ parent::__construct($name, $options);
+ }
+
+ /**
+ * Returns users for whom exceptions can be set/unset in the associated course.
+ *
+ * This method is used both when getting the list of choices to display to the user, and also when validating a list of users
+ * that was selected.
+ *
+ * @param string $search the search string.
+ * @return object[][] An array of arrays of users. The outer array will only have one element with an option group name as key.
+ * The keys of the inner array will be user IDs and the values will be the corresponding user objects
+ * containing the fields returned by the method {@see required_fields_sql}.
+ * If no users match the provided `$search` string, an empty array is returned instead (without nesting).
+ * @throws coding_exception
+ * @throws dml_exception
+ */
+ public function find_users($search): array {
+ global $DB;
+ if (!$this->is_validating()) {
+ // To avoid returning too many matching user records, we count them first.
+ [$countsql, $countparams] = $this->build_find_users_query($search, selectcount: true);
+ $userscount = $DB->count_records_sql($countsql, $countparams);
+ if ($userscount > $this->maxusersperpage) {
+ return $this->too_many_results($search, $userscount);
+ }
+ }
+ [$sql, $params] = $this->build_find_users_query($search);
+ $users = $DB->get_records_sql($sql, $params);
+ if (empty($users)) {
+ return [];
+ }
+ $stringkey = $this->skipauth ? 'users_with_exception' : 'users_without_exception';
+ if ($search) {
+ $groupname = get_string("{$stringkey}_matching", 'availability_shibboleth2fa', $search);
+ } else {
+ $groupname = get_string($stringkey, 'availability_shibboleth2fa');
+ }
+ return [$groupname => $users];
+ }
+
+ /**
+ * Constructs and returns an SQL query along with the corresponding parameters for finding users via the selector.
+ *
+ * @param string $search Search string to find users by.
+ * @param bool $selectcount Whether the resulting query is merely for counting the number of matching records.
+ * If `false` (default), the query will select the fields returned by {@see required_fields_sql}.
+ * @return array Two items, the first being the SQL query string, and the second an array of the corresponding parameters.
+ * @throws coding_exception
+ */
+ protected function build_find_users_query(string $search, bool $selectcount = false): array {
+ // By default, `wherecondition` retrieves all users except the deleted, not confirmed and guest.
+ [$wherecondition, $params] = $this->search_sql($search, 'u');
+ $params['courseid'] = $this->courseid;
+ // Different queries depending on whether we want to search among users with or without existing exceptions in the course.
+ if ($this->skipauth) {
+ $ejoin = 'JOIN';
+ } else {
+ $ejoin = 'LEFT JOIN';
+ $wherecondition .= ' AND e.id IS NULL';
+ }
+ if ($selectcount) {
+ $fields = 'COUNT(1)';
+ $orderby = '';
+ } else {
+ $fields = $this->required_fields_sql('u');
+ [$sort, $sortparams] = users_order_by_sql('u', $search, $this->accesscontext);
+ $orderby = "ORDER BY $sort";
+ $params += $sortparams;
+ }
+ $sql = "SELECT $fields
+ FROM {user} u
+ JOIN {user_enrolments} AS ue ON ue.userid = u.id
+ JOIN {enrol} AS en ON ue.enrolid = en.id
+ JOIN {course} AS course ON en.courseid = course.id
+ $ejoin {availability_shibboleth2fa_e} e ON (e.userid = u.id AND e.courseid = course.id AND e.skipauth = 1)
+ WHERE $wherecondition
+ AND course.id = :courseid
+ $orderby";
+ return [$sql, $params];
+ }
+
+ /**
+ * Returns the options needed to recreate the given {@see user_exception_selector}.
+ *
+ * @return array Options to be passed to the class' constructor.
+ */
+ protected function get_options(): array {
+ return parent::get_options() + [
+ 'courseid' => $this->courseid,
+ 'skipauth' => $this->skipauth,
+ ];
+ }
+
+ /**
+ * If any users have been selected, an exception for them is added/removed in the associated course.
+ *
+ * If the selector was created for users that are already exempt from 2FA requirements in the course (`skipauth` set to `true`),
+ * the exceptions will be removed for the selected users. If it was created for users that are not (yet) exempt from 2FA
+ * requirements in the course (`skipauth` set to `false`), exceptions will be added for the selected users.
+ *
+ * Calls the {@see invalidate_selected_users} method at the end.
+ *
+ * @return object[] The selected users for whom the exception status has been modified.
+ * @throws dml_exception
+ */
+ public function set_exceptions_for_selected_users(): array {
+ $users = $this->get_selected_users();
+ foreach ($users as $user) {
+ condition::set_exception(
+ courseid: $this->courseid,
+ userid: $user->id,
+ skipauth: !$this->skipauth,
+ );
+ }
+ $this->invalidate_selected_users();
+ return $users;
+ }
+}
diff --git a/manage.php b/manage.php
index 4d06a29..ca196e5 100644
--- a/manage.php
+++ b/manage.php
@@ -20,13 +20,11 @@
* @package availability_shibboleth2fa
* @copyright 2021 Lars Bonczek, innoCampus, TU Berlin
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
- * 4
+ *
* {@noinspection PhpUnhandledExceptionInspection}
*/
-use availability_shibboleth2fa\condition;
-use availability_shibboleth2fa\exception_current_user_selector;
-use availability_shibboleth2fa\exception_potential_user_selector;
+use availability_shibboleth2fa\user_exception_selector;
require(__DIR__ . '/../../../config.php');
@@ -40,60 +38,35 @@
require_login($course, false);
-$context = context_course::instance($course->id);
-
-require_capability('availability/shibboleth2fa:manageexceptions', $context);
+require_capability('availability/shibboleth2fa:manageexceptions', context_course::instance($courseid));
$PAGE->set_title(get_string('fulltitle', 'availability_shibboleth2fa'));
$PAGE->set_heading($course->fullname);
echo $OUTPUT->header();
-// Create the user selector objects.
-$options = ['courseid' => $course->id, 'accesscontext' => $context];
+$potentialuserselector = user_exception_selector::instance($courseid, skipauth: false);
+$currentuserselector = user_exception_selector::instance($courseid, skipauth: true);
-$potentialuserselector = new exception_potential_user_selector('addselect', $options);
-$currentuserselector = new exception_current_user_selector('removeselect', $options);
-
-// Process add and removes.
+// Add/remove user exceptions.
+// Checking which of the two submit buttons was pressed (`add` or `remove`) ensures only one of the two corresponding actions will
+// be performed following the form submission. Even if users in both selectors had been selected, when the `add` button was pressed,
+// those selected in the `$currentuserselector` will not have their exceptions removed, until the `remove` button is subsequently
+// pressed as well, and vice versa.
if (optional_param('add', false, PARAM_BOOL) && confirm_sesskey()) {
- $userstoassign = $potentialuserselector->get_selected_users();
- if (!empty($userstoassign)) {
- foreach ($userstoassign as $adduser) {
- condition::set_exception(
- courseid: $course->id,
- userid: $adduser->id,
- skipauth: true,
- );
- }
- $potentialuserselector->invalidate_selected_users();
- $currentuserselector->invalidate_selected_users();
- }
+ $potentialuserselector->set_exceptions_for_selected_users();
}
-
-// Process incoming role unassignments.
if (optional_param('remove', false, PARAM_BOOL) && confirm_sesskey()) {
- $userstounassign = $currentuserselector->get_selected_users();
- if (!empty($userstounassign)) {
- foreach ($userstounassign as $removeuser) {
- condition::set_exception(
- courseid: $course->id,
- userid: $removeuser->id,
- skipauth: false,
- );
- }
- $potentialuserselector->invalidate_selected_users();
- $currentuserselector->invalidate_selected_users();
- }
+ $currentuserselector->set_exceptions_for_selected_users();
}
echo $OUTPUT->heading(get_string('user_exceptions', 'availability_shibboleth2fa'));
$templatecontext = [
- 'actionurl' => $PAGE->url,
- 'sesskey' => sesskey(),
- 'currentuserselector' => $currentuserselector->display(return: true),
+ 'actionurl' => $PAGE->url,
+ 'sesskey' => sesskey(),
+ 'currentuserselector' => $currentuserselector->display(return: true),
'potentialuserselector' => $potentialuserselector->display(return: true),
- 'larrow' => $OUTPUT->larrow(),
- 'rarrow' => $OUTPUT->rarrow(),
+ 'larrow' => $OUTPUT->larrow(),
+ 'rarrow' => $OUTPUT->rarrow(),
];
echo $OUTPUT->render_from_template('availability_shibboleth2fa/manage_form', $templatecontext);
echo $OUTPUT->footer();
diff --git a/templates/manage_form.mustache b/templates/manage_form.mustache
index 3c4d509..af4637c 100644
--- a/templates/manage_form.mustache
+++ b/templates/manage_form.mustache
@@ -35,12 +35,12 @@
Example context (json):
{
- "actionurl": "/availability/condition/shibboleth2fa/manage.php?id=123",
- "sesskey": "abc123",
- "currentuserselector": "",
+ "actionurl": "/availability/condition/shibboleth2fa/manage.php?id=123",
+ "sesskey": "abc123",
+ "currentuserselector": "",
"potentialuserselector": "",
- "larrow": "◄",
- "rarrow": "►"
+ "larrow": "◄",
+ "rarrow": "►"
}
}}