MDL-62309 tool_policy: Improve permission evaluation methods

Please refer to the unit tests for the specification of the expected
behaviour.
This commit is contained in:
David Mudrák
2018-10-22 08:49:36 +02:00
parent 4a742e4f94
commit ad5e2135c5
10 changed files with 301 additions and 55 deletions
+86 -21
View File
@@ -780,14 +780,19 @@ class api {
}
/**
* Checks if user can accept policies for themselves or on behalf of another user
* Check if given policies can be accepted by the current user (eventually on behalf of the other user)
*
* @param int $userid
* @param bool $throwexception
* Currently, the version ids are not relevant and the check is based on permissions only. In the future, additional
* conditions can be added (such as policies applying to certain users only).
*
* @param array $versionids int[] List of policy version ids to check
* @param int $userid Accepting policies on this user's behalf (defaults to accepting on self)
* @param bool $throwexception Throw exception instead of returning false
* @return bool
*/
public static function can_accept_policies($userid = null, $throwexception = false) {
public static function can_accept_policies(array $versionids, $userid = null, $throwexception = false) {
global $USER;
if (!isloggedin() || isguestuser()) {
if ($throwexception) {
throw new \moodle_exception('noguest');
@@ -795,6 +800,7 @@ class api {
return false;
}
}
if (!$userid) {
$userid = $USER->id;
}
@@ -820,15 +826,48 @@ class api {
}
/**
* Checks if user can revoke policies for themselves or on behalf of another user
* Check if given policies can be declined by the current user (eventually on behalf of the other user)
*
* @param int $userid
* @param bool $throwexception
* Only optional policies can be declined. Otherwise, the permissions are same as for accepting policies.
*
* @param array $versionids int[] List of policy version ids to check
* @param int $userid Declining policies on this user's behalf (defaults to declining by self)
* @param bool $throwexception Throw exception instead of returning false
* @return bool
*/
public static function can_revoke_policies($userid = null, $throwexception = false) {
public static function can_decline_policies(array $versionids, $userid = null, $throwexception = false) {
foreach ($versionids as $versionid) {
if (static::get_agreement_optional($versionid) == policy_version::AGREEMENT_COMPULSORY) {
// Compulsory policies can't be declined (that is what makes them compulsory).
if ($throwexception) {
throw new \moodle_exception('errorpolicyversioncompulsory', 'tool_policy');
} else {
return false;
}
}
}
return static::can_accept_policies($versionids, $userid, $throwexception);
}
/**
* Check if acceptances to given policies can be revoked by the current user (eventually on behalf of the other user)
*
* Revoking optional policies is controlled by the same rules as declining them. Compulsory policies can be revoked
* only by users with the permission to accept policies on other's behalf. The reasoning behind this is to make sure
* the user communicates with the site's privacy officer and is well aware of all consequences of the decision (such
* as losing right to access the site).
*
* @param array $versionids int[] List of policy version ids to check
* @param int $userid Revoking policies on this user's behalf (defaults to revoking by self)
* @param bool $throwexception Throw exception instead of returning false
* @return bool
*/
public static function can_revoke_policies(array $versionids, $userid = null, $throwexception = false) {
global $USER;
// Guests' acceptance is not stored so there is nothing to revoke.
if (!isloggedin() || isguestuser()) {
if ($throwexception) {
throw new \moodle_exception('noguest');
@@ -836,20 +875,46 @@ class api {
return false;
}
}
if (!$userid) {
$userid = $USER->id;
// Sort policies into two sets according the optional flag.
$compulsory = [];
$optional = [];
foreach ($versionids as $versionid) {
$agreementoptional = static::get_agreement_optional($versionid);
if ($agreementoptional == policy_version::AGREEMENT_COMPULSORY) {
$compulsory[] = $versionid;
} else if ($agreementoptional == policy_version::AGREEMENT_OPTIONAL) {
$optional[] = $versionid;
} else {
throw new \coding_exception('Unexpected optional flag value');
}
}
// At the moment, current users can't revoke their own policies.
// Check capability to revoke on behalf as the real user.
$realuser = manager::get_realuser();
$usercontext = \context_user::instance($userid);
if ($throwexception) {
require_capability('tool/policy:acceptbehalf', $usercontext, $realuser);
return;
} else {
return has_capability('tool/policy:acceptbehalf', $usercontext, $realuser);
// Check if the user can revoke the optional policies from the list.
if ($optional) {
if (!static::can_decline_policies($optional, $userid, $throwexception)) {
return false;
}
}
// Check if the user can revoke the compulsory policies from the list.
if ($compulsory) {
if (!$userid) {
$userid = $USER->id;
}
$realuser = manager::get_realuser();
$usercontext = \context_user::instance($userid);
if ($throwexception) {
require_capability('tool/policy:acceptbehalf', $usercontext, $realuser);
return;
} else {
return has_capability('tool/policy:acceptbehalf', $usercontext, $realuser);
}
}
return true;
}
/**
@@ -897,7 +962,7 @@ class api {
if (!$userid) {
$userid = $USER->id;
}
self::can_accept_policies($userid, true);
self::can_accept_policies([$policyversionid], $userid, true);
// Retrieve the list of policy versions that need agreement (do not update existing agreements).
list($sql, $params) = $DB->get_in_or_equal($policyversionid, SQL_PARAMS_NAMED);
@@ -986,7 +1051,7 @@ class api {
if (!$userid) {
$userid = $USER->id;
}
self::can_accept_policies($userid, true);
self::can_accept_policies([$policyversionid], $userid, true);
if ($currentacceptance = $DB->get_record('tool_policy_acceptances',
['policyversionid' => $policyversionid, 'userid' => $userid])) {
@@ -56,7 +56,7 @@ class accept_policy extends \moodleform {
$action = $this->_customdata['action'];
$userids = clean_param_array($this->_customdata['userids'], PARAM_INT);
$versionids = clean_param_array($this->_customdata['versionids'], PARAM_INT);
$usernames = $this->validate_and_get_users($userids, $action);
$usernames = $this->validate_and_get_users($versionids, $userids, $action);
$versionnames = $this->validate_and_get_versions($versionids);
foreach ($usernames as $userid => $name) {
@@ -117,11 +117,12 @@ class accept_policy extends \moodleform {
/**
* Validate userids and return usernames
*
* @param array $versionids int[] List of policy version ids to process.
* @param array $userids
* @param string $action accept|decline|revoke
* @return array (userid=>username)
*/
protected function validate_and_get_users($userids, $action) {
protected function validate_and_get_users($versionids, $userids, $action) {
global $DB;
$usernames = [];
@@ -142,11 +143,11 @@ class accept_policy extends \moodleform {
}
\context_helper::preload_from_record($user);
if ($action === 'revoke') {
api::can_revoke_policies($userid, true);
api::can_revoke_policies($versionids, $userid, true);
} else if ($action === 'accept') {
api::can_accept_policies($userid, true);
api::can_accept_policies($versionids, $userid, true);
} else if ($action === 'decline') {
api::can_decline_policies($userid, true);
api::can_decline_policies($versionids, $userid, true);
}
$usernames[$userid] = fullname($user);
}
@@ -50,9 +50,6 @@ class acceptances implements renderable, templatable {
/** @var moodle_url */
protected $returnurl;
/** @var bool */
protected $canrevoke;
/**
* Contructor.
*
@@ -62,7 +59,6 @@ class acceptances implements renderable, templatable {
public function __construct($userid, $returnurl = null) {
$this->userid = $userid;
$this->returnurl = $returnurl ? (new moodle_url($returnurl))->out(false) : null;
$this->canrevoke = \tool_policy\api::can_revoke_policies($this->userid);
}
/**
@@ -76,19 +72,20 @@ class acceptances implements renderable, templatable {
$data->hasonbehalfagreements = false;
$data->pluginbaseurl = (new moodle_url('/admin/tool/policy'))->out(false);
$data->returnurl = $this->returnurl;
$data->canrevoke = $this->canrevoke;
// Get the list of policies and versions that current user is able to see
// and the respective acceptance records for the selected user.
$policies = api::get_policies_with_acceptances($this->userid);
$versionids = [];
$canviewfullnames = has_capability('moodle/site:viewfullnames', \context_system::instance());
foreach ($policies as $policy) {
foreach ($policy->versions as $version) {
$versionids[$version->id] = $version->id;
unset($version->summary);
unset($version->content);
$version->iscurrent = ($version->status == policy_version::STATUS_ACTIVE);
$version->isoptional = ($version->optional == policy_version::AGREEMENT_OPTIONAL);
$version->name = $version->name;
$version->revision = $version->revision;
$returnurl = new moodle_url('/admin/tool/policy/user.php', ['userid' => $this->userid]);
@@ -138,6 +135,8 @@ class acceptances implements renderable, templatable {
}
$data->policies = array_values($policies);
$data->canrevoke = \tool_policy\api::can_revoke_policies(array_keys($versionids), $this->userid);
return $data;
}
}
@@ -352,7 +352,7 @@ class page_agreedocs implements renderable, templatable {
// Check for correct user capabilities.
if ($this->isexistinguser) {
// For existing users, it's needed to check if they have the capability for accepting policies.
api::can_accept_policies($this->behalfid, true);
api::can_accept_policies($this->listdocs, $this->behalfid, true);
} else {
// For new users, the behalfid parameter is ignored.
if ($this->behalfid) {
@@ -65,9 +65,10 @@ class page_nopermission implements renderable, templatable {
/**
* Prepare the page for rendering.
*
* @param array $versionids int[] List of policy version ids that were checked.
* @param int $behalfid The userid to consent policies as (such as child's id).
*/
public function __construct($behalfid) {
public function __construct(array $versionids, $behalfid) {
global $USER;
$behalfid = $behalfid ?: $USER->id;
@@ -79,7 +80,7 @@ class page_nopermission implements renderable, templatable {
if (!empty($USER->id)) {
// For existing users, it's needed to check if they have the capability for accepting policies.
$this->haspermissionagreedocs = api::can_accept_policies($this->behalfid);
$this->haspermissionagreedocs = api::can_accept_policies($versionids, $this->behalfid);
}
$this->policies = api::list_current_versions(policy_version::AUDIENCE_LOGGEDIN);
@@ -94,11 +94,11 @@ class user_agreement implements \templatable, \renderable {
$this->canaccept = $canaccept;
if (count($this->accepted) < count($this->versions) && $canaccept === null) {
$this->canaccept = \tool_policy\api::can_accept_policies($this->userid);
$this->canaccept = \tool_policy\api::can_accept_policies(array_keys($this->versions), $this->userid);
}
if (count($this->accepted) > 0 && $canrevoke === null) {
$this->canrevoke = \tool_policy\api::can_revoke_policies($this->userid);
$this->canrevoke = \tool_policy\api::can_revoke_policies(array_keys($this->versions), $this->userid);
}
}
+2 -2
View File
@@ -68,14 +68,14 @@ if (array_diff($agreedocs, $listdocs) || array_diff($declinedocs, $listdocs)) {
if (isloggedin() && !isguestuser()) {
// Existing user.
$haspermissionagreedocs = api::can_accept_policies($behalfid);
$haspermissionagreedocs = api::can_accept_policies($listdocs, $behalfid);
} else {
// New user.
$haspermissionagreedocs = true;
}
if (!$haspermissionagreedocs) {
$outputpage = new \tool_policy\output\page_nopermission($behalfid);
$outputpage = new \tool_policy\output\page_nopermission($listdocs, $behalfid);
} else if ($cancel) {
redirect(new moodle_url('/'));
} else {
@@ -55,6 +55,7 @@ $string['declinethepolicy'] = 'Decline user consent';
$string['deleting'] = 'Deleting a version';
$string['deleteconfirm'] = '<p>Are you sure you want to delete policy <em>\'{$a->name}\'</em>?</p><p>This operation can not be undone.</p>';
$string['editingpolicydocument'] = 'Editing policy';
$string['errorpolicyversioncompulsory'] = 'Compulsory policies cannot be declined!';
$string['errorpolicyversionnotfound'] = 'There isn\'t any policy version with this identifier.';
$string['errorsaveasdraft'] = 'Minor change can not be saved as draft';
$string['errorusercantviewpolicyversion'] = 'The user doesn\'t have access to this policy version.';
@@ -43,6 +43,8 @@
"revision": "2.0",
"hasarchived": true,
"timeaccepted": "1 Mar 2018",
"iscurrent": true,
"isoptional": false,
"agreement": {
"onbehalf": false,
"status": false,
@@ -60,6 +62,8 @@
"note": "Based on parent's agreement via email",
"hasarchived": false,
"timeaccepted": "15 Feb 2018",
"iscurrent": true,
"isoptional": false,
"agreement": {
"onbehalf": true,
"status": true,
@@ -106,7 +110,8 @@
</td>
<td>
<a href="{{viewurl}}">{{{revision}}}</a>
{{#iscurrent}}<span class="label label-success">{{#str}} status1, tool_policy {{/str}}{{/iscurrent}}
{{#iscurrent}}<span class="label label-success">{{#str}} status1, tool_policy {{/str}}</span>{{/iscurrent}}
{{#isoptional}}<span class="label label-info">{{#str}} policydocoptionalyes, tool_policy {{/str}}</span>{{/isoptional}}
</td>
<td>
{{>tool_policy/user_agreement}}
+189 -15
View File
@@ -368,6 +368,164 @@ class tool_policy_api_testcase extends advanced_testcase {
$this->assertTrue(api::can_user_view_policy_version($policy3, null, $parent->id));
}
/**
* Test behaviour of the {@link api::can_accept_policies()} method.
*/
public function test_can_accept_policies() {
global $CFG;
$this->resetAfterTest();
$this->setAdminUser();
$user = $this->getDataGenerator()->create_user();
$child = $this->getDataGenerator()->create_user();
$parent = $this->getDataGenerator()->create_user();
$officer = $this->getDataGenerator()->create_user();
$manager = $this->getDataGenerator()->create_user();
$syscontext = context_system::instance();
$childcontext = context_user::instance($child->id);
$roleminorid = create_role('Digital minor', 'digiminor', 'Not old enough to accept site policies themselves');
$roleparentid = create_role('Parent', 'parent', 'Can accept policies on behalf of their child');
$roleofficerid = create_role('Policy officer', 'policyofficer', 'Can see all acceptances but can\'t edit policy documents');
$rolemanagerid = create_role('Policy manager', 'policymanager', 'Can manage policy documents');
assign_capability('tool/policy:accept', CAP_PROHIBIT, $roleminorid, $syscontext->id);
assign_capability('tool/policy:acceptbehalf', CAP_ALLOW, $roleparentid, $syscontext->id);
assign_capability('tool/policy:acceptbehalf', CAP_ALLOW, $roleofficerid, $syscontext->id);
assign_capability('tool/policy:viewacceptances', CAP_ALLOW, $roleofficerid, $syscontext->id);
assign_capability('tool/policy:acceptbehalf', CAP_ALLOW, $rolemanagerid, $syscontext->id);
assign_capability('tool/policy:managedocs', CAP_ALLOW, $rolemanagerid, $syscontext->id);
role_assign($roleminorid, $child->id, $syscontext->id);
role_assign($roleparentid, $parent->id, $childcontext->id);
role_assign($roleofficerid, $officer->id, $syscontext->id);
role_assign($rolemanagerid, $manager->id, $syscontext->id);
accesslib_clear_all_caches_for_unit_testing();
$policy1 = $this->add_policy(['optional' => policy_version::AGREEMENT_COMPULSORY])->to_record();
$policy2 = $this->add_policy(['optional' => policy_version::AGREEMENT_COMPULSORY])->to_record();
$policy3 = $this->add_policy(['optional' => policy_version::AGREEMENT_OPTIONAL])->to_record();
$policy4 = $this->add_policy(['optional' => policy_version::AGREEMENT_OPTIONAL])->to_record();
$mixed = [$policy1->id, $policy2->id, $policy3->id, $policy4->id];
$compulsory = [$policy1->id, $policy2->id];
$optional = [$policy3->id, $policy4->id];
// Normally users can accept all policies.
$this->setUser($user);
$this->assertTrue(api::can_accept_policies($mixed));
$this->assertTrue(api::can_accept_policies($compulsory));
$this->assertTrue(api::can_accept_policies($optional));
// Digital minors can be set to not be able to accept policies themselves.
$this->setUser($child);
$this->assertFalse(api::can_accept_policies($mixed));
$this->assertFalse(api::can_accept_policies($compulsory));
$this->assertFalse(api::can_accept_policies($optional));
// The parent can accept optional policies on child's behalf.
$this->setUser($parent);
$this->assertTrue(api::can_accept_policies($mixed, $child->id));
$this->assertTrue(api::can_accept_policies($compulsory, $child->id));
$this->assertTrue(api::can_accept_policies($optional, $child->id));
// Officers and managers can accept on other user's behalf.
$this->setUser($officer);
$this->assertTrue(api::can_accept_policies($mixed, $parent->id));
$this->assertTrue(api::can_accept_policies($compulsory, $parent->id));
$this->assertTrue(api::can_accept_policies($optional, $parent->id));
$this->setUser($manager);
$this->assertTrue(api::can_accept_policies($mixed, $parent->id));
$this->assertTrue(api::can_accept_policies($compulsory, $parent->id));
$this->assertTrue(api::can_accept_policies($optional, $parent->id));
}
/**
* Test behaviour of the {@link api::can_decline_policies()} method.
*/
public function test_can_decline_policies() {
global $CFG;
$this->resetAfterTest();
$this->setAdminUser();
$user = $this->getDataGenerator()->create_user();
$child = $this->getDataGenerator()->create_user();
$parent = $this->getDataGenerator()->create_user();
$officer = $this->getDataGenerator()->create_user();
$manager = $this->getDataGenerator()->create_user();
$syscontext = context_system::instance();
$childcontext = context_user::instance($child->id);
$roleminorid = create_role('Digital minor', 'digiminor', 'Not old enough to accept site policies themselves');
$roleparentid = create_role('Parent', 'parent', 'Can accept policies on behalf of their child');
$roleofficerid = create_role('Policy officer', 'policyofficer', 'Can see all acceptances but can\'t edit policy documents');
$rolemanagerid = create_role('Policy manager', 'policymanager', 'Can manage policy documents');
assign_capability('tool/policy:accept', CAP_PROHIBIT, $roleminorid, $syscontext->id);
assign_capability('tool/policy:acceptbehalf', CAP_ALLOW, $roleparentid, $syscontext->id);
assign_capability('tool/policy:acceptbehalf', CAP_ALLOW, $roleofficerid, $syscontext->id);
assign_capability('tool/policy:viewacceptances', CAP_ALLOW, $roleofficerid, $syscontext->id);
assign_capability('tool/policy:acceptbehalf', CAP_ALLOW, $rolemanagerid, $syscontext->id);
assign_capability('tool/policy:managedocs', CAP_ALLOW, $rolemanagerid, $syscontext->id);
role_assign($roleminorid, $child->id, $syscontext->id);
role_assign($roleparentid, $parent->id, $childcontext->id);
role_assign($roleofficerid, $officer->id, $syscontext->id);
role_assign($rolemanagerid, $manager->id, $syscontext->id);
accesslib_clear_all_caches_for_unit_testing();
$policy1 = $this->add_policy(['optional' => policy_version::AGREEMENT_COMPULSORY])->to_record();
$policy2 = $this->add_policy(['optional' => policy_version::AGREEMENT_COMPULSORY])->to_record();
$policy3 = $this->add_policy(['optional' => policy_version::AGREEMENT_OPTIONAL])->to_record();
$policy4 = $this->add_policy(['optional' => policy_version::AGREEMENT_OPTIONAL])->to_record();
$mixed = [$policy1->id, $policy2->id, $policy3->id, $policy4->id];
$compulsory = [$policy1->id, $policy2->id];
$optional = [$policy3->id, $policy4->id];
// Normally users can decline only optional policies.
$this->setUser($user);
$this->assertFalse(api::can_decline_policies($mixed));
$this->assertFalse(api::can_decline_policies($compulsory));
$this->assertTrue(api::can_decline_policies($optional));
// If they can't accept them, they can't decline them too.
$this->setUser($child);
$this->assertFalse(api::can_decline_policies($mixed));
$this->assertFalse(api::can_decline_policies($compulsory));
$this->assertFalse(api::can_decline_policies($optional));
// The parent can decline optional policies on child's behalf.
$this->setUser($parent);
$this->assertFalse(api::can_decline_policies($mixed, $child->id));
$this->assertFalse(api::can_decline_policies($compulsory, $child->id));
$this->assertTrue(api::can_decline_policies($optional, $child->id));
// Even officers or managers cannot decline compulsory policies.
$this->setUser($officer);
$this->assertFalse(api::can_decline_policies($mixed));
$this->assertFalse(api::can_decline_policies($compulsory));
$this->assertTrue(api::can_decline_policies($optional));
$this->assertFalse(api::can_decline_policies($mixed, $child->id));
$this->assertFalse(api::can_decline_policies($compulsory, $child->id));
$this->assertTrue(api::can_decline_policies($optional, $child->id));
$this->setUser($manager);
$this->assertFalse(api::can_decline_policies($mixed));
$this->assertFalse(api::can_decline_policies($compulsory));
$this->assertTrue(api::can_decline_policies($optional));
$this->assertFalse(api::can_decline_policies($mixed, $child->id));
$this->assertFalse(api::can_decline_policies($compulsory, $child->id));
$this->assertTrue(api::can_decline_policies($optional, $child->id));
}
/**
* Test behaviour of the {@link api::can_revoke_policies()} method.
*/
@@ -406,32 +564,48 @@ class tool_policy_api_testcase extends advanced_testcase {
accesslib_clear_all_caches_for_unit_testing();
// Prepare a policy document with some versions.
list($policy1, $policy2, $policy3) = $this->create_versions(3);
$policy1 = helper::add_policy(['optional' => policy_version::AGREEMENT_COMPULSORY])->to_record();
$policy2 = helper::add_policy(['optional' => policy_version::AGREEMENT_OPTIONAL])->to_record();
$versionids = [$policy1->id, $policy2->id];
// Guests cannot revoke anything.
$this->setGuestUser();
$this->assertFalse(api::can_revoke_policies($versionids));
// Normally users do not have access to revoke policies.
$this->setUser($user);
$this->assertFalse(api::can_revoke_policies($user->id));
$this->assertFalse(api::can_revoke_policies($versionids, $user->id));
$this->setUser($child);
$this->assertFalse(api::can_revoke_policies($child->id));
$this->assertFalse(api::can_revoke_policies($versionids, $child->id));
// The parent can revoke the policy on behalf of her child (but not her own policies).
// Optional policies can be revoked if the user can accept them.
$this->setUser($user);
$this->assertTrue(api::can_revoke_policies([$policy2->id]));
$this->assertTrue(api::can_revoke_policies([$policy2->id], $user->id));
$this->setUser($child);
$this->assertFalse(api::can_revoke_policies([$policy2->id]));
$this->assertFalse(api::can_revoke_policies([$policy2->id], $child->id));
// The parent can revoke the policy on behalf of her child (but not her own policies, unless they are optional).
$this->setUser($parent);
$this->assertFalse(api::can_revoke_policies($parent->id));
$this->assertTrue(api::can_revoke_policies($child->id));
$this->assertFalse(api::can_revoke_policies($versionids, $parent->id));
$this->assertTrue(api::can_revoke_policies($versionids, $child->id));
$this->assertTrue(api::can_revoke_policies([$policy2->id]));
$this->assertTrue(api::can_revoke_policies([$policy2->id], $child->id));
// Officers and managers can revoke everything.
$this->setUser($officer);
$this->assertTrue(api::can_revoke_policies($officer->id));
$this->assertTrue(api::can_revoke_policies($child->id));
$this->assertTrue(api::can_revoke_policies($parent->id));
$this->assertTrue(api::can_revoke_policies($manager->id));
$this->assertTrue(api::can_revoke_policies($versionids, $officer->id));
$this->assertTrue(api::can_revoke_policies($versionids, $child->id));
$this->assertTrue(api::can_revoke_policies($versionids, $parent->id));
$this->assertTrue(api::can_revoke_policies($versionids, $manager->id));
$this->setUser($manager);
$this->assertTrue(api::can_revoke_policies($manager->id));
$this->assertTrue(api::can_revoke_policies($child->id));
$this->assertTrue(api::can_revoke_policies($parent->id));
$this->assertTrue(api::can_revoke_policies($officer->id));
$this->assertTrue(api::can_revoke_policies($versionids, $manager->id));
$this->assertTrue(api::can_revoke_policies($versionids, $child->id));
$this->assertTrue(api::can_revoke_policies($versionids, $parent->id));
$this->assertTrue(api::can_revoke_policies($versionids, $officer->id));
}
/**