Commit 031a981c authored by Nigel McNie's avatar Nigel McNie
Browse files

Prevent role changing in groups if the user is the only admin. Add some...

Prevent role changing in groups if the user is the only admin. Add some functions to abstract role changing.

This patch adds:

 * group_is_only_admin($group, $user)
 * group_can_change_role($group, $user, $newrole)
 * group_change_role($group, $user, $newrole)

These functions are used on changerole.php, and also on the member listing page to prevent showing the change role link if the user is the only admin in the group.

If the user has no roles that they can change to and changerole.php is visited, it will redirect back to the member listing with an informational message.
parent fc3689a7
......@@ -50,13 +50,23 @@ if ($role != 'admin') {
}
$roles = group_get_role_info($groupid);
$rolechange_available = false;
foreach ($roles as &$r) {
$disabled = ($r->role == $currentrole) || (!group_can_change_role($groupid, $userid, $r->role));
if (!$disabled) {
$rolechange_available = true;
}
$r = array(
'value' => $r->display,
'disabled' => $r->role == $currentrole,
'value' => $r->display,
'disabled' => $disabled,
);
}
if (!$rolechange_available) {
$SESSION->add_info_msg('This user has no roles they can change to');
redirect('/group/members.php?id=' . $groupid);
}
$changeform = pieform(array(
'name' => 'changerole',
'method' => 'post',
......@@ -75,12 +85,17 @@ $changeform = pieform(array(
)
));
function changerole_submit(Pieform $form, $values) {
global $user, $group, $currentrole, $SESSION;
if ($values['role'] && $values['role'] != $currentrole) {
set_field('group_member', 'role', $values['role'], 'group', $group->id, 'member', $user->id);
$SESSION->add_ok_msg(get_string('rolechanged', 'group'));
function changerole_validate(Pieform $form, $values) {
global $user, $group;
if (!group_can_change_role($group->id, $user->id, $values['role'])) {
$form->set_error('role', get_string('usercannotchangetothisrole', 'group'));
}
}
function changerole_submit(Pieform $form, $values) {
global $user, $group, $SESSION;
group_change_role($group->id, $user->id, $values['role']);
$SESSION->add_ok_msg(get_string('rolechanged', 'group'));
redirect('/group/members.php?id='.$group->id);
}
......
......@@ -130,6 +130,7 @@ $string['groupconfirmleave'] = 'Are you sure you want to leave this group?';
$string['groupconfirmleavehasviews'] = 'Are you sure you want to leave this group? Some of your views use this group for access control, leaving this group would mean that the members of the group would not have access to the views';
$string['cantleavegroup'] = 'You can\'t leave this group';
$string['usercantleavegroup'] = 'This user cannot leave this group';
$string['usercannotchangetothisrole'] = 'The user cannot change to this role';
$string['leavespecifiedgroup'] = 'Leave group \'%s\'';
$string['memberslist'] = 'Members: ';
$string['nogroups'] = 'No groups';
......
......@@ -54,8 +54,7 @@ function group_user_can_leave($group, $userid=null) {
return ($result[$group->id][$userid] = false);
}
if (group_user_access($group->id, $userid) == 'admin'
&& count_records('group_member', 'group', $group->id, 'role', 'admin') == 1) {
if (group_is_only_admin($group->id, $userid)) {
return ($result[$group->id][$userid] = false);
}
......@@ -164,6 +163,66 @@ function group_user_access($groupid, $userid=null) {
return get_field('group_member', 'role', 'group', $groupid, 'member', $userid);
}
/**
* Returns whether the given user is the only administrator in the given group.
*
* If the user isn't in the group, or they're not an admin, or there is another admin, false
* is returned.
*
* @param int $group The ID of the group to check
* @param int $user The ID of the user to check
* @returns boolean
*/
function group_is_only_admin($group, $user) {
return group_user_access($group, $user) == 'admin'
&& count_records('group_member', 'group', $group, 'role', 'admin') == 1;
}
/**
* Returns whether the given user is allowed to change their role to the
* requested role in the given group.
*
* This function is checking whether _role changes_ are allowed, not if a user
* is allowed to be added to a group.
*
* @param int $group The ID of the group to check
* @param int $user The ID of the user to check
* @param string $role The role the user wishes to switch to
* @returns boolean
*/
function group_can_change_role($group, $user, $role) {
if (!group_user_access($group, $user)) {
return false;
}
// Sole remaining admins can never change their role
if (group_is_only_admin($group, $user)) {
return false;
}
// Maybe one day more checks will be needed - they go here
return true;
}
/**
* Changes a user role in a group, if this is allowed.
*
* @param int $group The ID of the group
* @param int $user The ID of the user whose role needs changing
* @param string $role The role the user wishes to switch to
* @throws AccessDeniedException If the specified role change is not allowed.
* Check with group_can_change_role first if you
* need to.
*/
function group_change_role($group, $user, $role) {
if (!group_can_change_role($group, $user, $role)) {
throw new AccessDeniedException('TODO');
}
set_field('group_member', 'role', $role, 'group', $group, 'member', $user);
}
function group_user_can_edit_views($groupid, $userid=null) {
$groupid = (int)$groupid;
......@@ -467,6 +526,14 @@ function group_get_membersearch_data($group, $query, $offset, $limit, $membershi
if (group_user_can_leave($group, $r['id'])) {
$r['removeform'] = group_get_removeuser_form($r['id'], $group);
}
// NOTE: this is a quick approximation. We should really check whether,
// for each role in the group, that the user can change to it (using
// group_can_change_role). This only controls whether the 'change
// role' link appears though, so it doesn't matter too much. If the
// user clicks on this link, changerole.php does the full check and
// sends them back here saying that the user has no roles they can
// change to anyway.
$r['canchangerole'] = !group_is_only_admin($group, $r['id']);
}
if (!empty($membershiptype)) {
......
......@@ -7,7 +7,7 @@
<h4><a href="{$WWWROOT}user/view.php?id={$r.id|escape}">{$r.name|escape}</a></h4>
{if $r.role}
<div class="removeform">
{$results.roles[$r.role]->display}{if $caneditroles} (<a href="{$WWWROOT}group/changerole.php?group={$group}&amp;user={$r.id}">{str tag=changerole section=group}</a>){/if}
{$results.roles[$r.role]->display}{if $caneditroles && $r.canchangerole} (<a href="{$WWWROOT}group/changerole.php?group={$group}&amp;user={$r.id}">{str tag=changerole section=group}</a>){/if}
{$r.removeform}
</div>
<p><strong>Joined:</strong> {$r.jointime}</p>
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment