Commit 1d526637 authored by Leigh Stoller's avatar Leigh Stoller

This started out as a simple change ...

I noticed that group_root could not delete users from projects. Seems
like we should allow that, but with the restriction that a group_root
cannot delete another group_root. Simple enough, right? Well thats not
how the permission system works; permission to do stuff to users is
based on who you are in the project, not who you are doing it to.

And then there are the subtle differences in permission handling between
the Classic interface and the Portal interface. And I am fully
unmotivated to fix anything in the Classic interface, hard to believe?

Anyway, most people are not going to notice anything since the bulk of
the changes affect sub groups. Sigh.
parent 5c53a981
......@@ -823,7 +823,7 @@ sub AccessCheck($$$)
$mintrust = PROJMEMBERTRUST_GROUPROOT();
}
elsif ($access_type == TB_PROJECT_DELUSER()) {
$mintrust = PROJMEMBERTRUST_PROJROOT();
$mintrust = PROJMEMBERTRUST_GROUPROOT();
}
else {
print "*** Invalid access type: $access_type!\n";
......
#!/usr/bin/perl -wT
#
# Copyright (c) 2000-2016 University of Utah and the Flux Group.
# Copyright (c) 2000-2017 University of Utah and the Flux Group.
#
# {{{EMULAB-LICENSE
#
......@@ -160,7 +160,9 @@ if (defined($pid)) {
#
if (! $this_user->IsAdmin()) {
if (! defined($project) ||
! $project->AccessCheck($this_user, TB_PROJECT_DELUSER)) {
! $project->AccessCheck($this_user, TB_PROJECT_DELUSER) ||
# This prevents group_root from deleting other group_roots or leader.
$project->Trust($target_user) >= $project->Trust($this_user)) {
fatal("You do not have permission to remove user $target_user!");
}
}
......
......@@ -286,6 +286,11 @@ function Do_MemberList()
$blob["approved"] = ($target_group->UserTrust($user) ==
TBDB_TRUSTSTRING_NONE ? 0 : 1);
# Can the current user delete this user from the group.
$blob["candelete"]=
(ISADMIN() ||
$target_group->CanDeleteUser($this_user, $user) ? 1 : 0);
$members[$user->uid()] = $blob;
}
foreach ($nonmemberlist as $user) {
......@@ -327,7 +332,7 @@ function Do_GroupProfile()
function Do_EditMembership()
{
global $this_user, $target_group, $ajax_args;
global $newTrustMap, $TB_PROJECT_ADDUSER;
global $newTrustMap, $TB_PROJECT_ADDUSER, $TB_PROJECT_BESTOWGROUPROOT;
if (CheckPageArgs()) {
return;
......@@ -382,11 +387,32 @@ function Do_EditMembership()
SPITAJAX_ERROR(-1, "User is still heading up experiments: $uid");
return -1;
}
# Check trust values unless removing from the group/project.
if ($action != "remove" &&
!array_key_exists($trust, $newTrustMap)) {
SPITAJAX_ERROR(-1, "Invalid privilege: $trust");
return -1;
if ($action == "remove") {
# Check permission to do this. This is really a project test.
if (!$target_group->CanDeleteUser($this_user, $target_user)) {
SPITAJAX_ERROR(-1, "Not allowed to remove user: $uid");
return -1;
}
}
else {
# Check trust values unless removing from the group/project.
if (!array_key_exists($trust, $newTrustMap)) {
SPITAJAX_ERROR(-1, "Invalid privilege: $trust");
return -1;
}
$trust = $newTrustMap[$trust];
#
# If changing to manager privs, then make sure current user can
# bestow manager.
#
if ($trust == TBDB_TRUSTSTRING_GROUPROOT && !ISADMIN() &&
!$target_group->AccessCheck($this_user,
$TB_PROJECT_BESTOWGROUPROOT)) {
SPITAJAX_ERROR(-1,
"You are not allowed to bestow manager privs");
return -1;
}
}
}
reset($users);
......@@ -425,6 +451,7 @@ function Do_EditPrivs()
{
global $this_user, $target_group, $ajax_args;
global $newTrustMap, $TB_PROJECT_EDITGROUP, $TBADMINGROUP;
global $TB_PROJECT_BESTOWGROUPROOT;
if (CheckPageArgs()) {
return;
......@@ -460,6 +487,16 @@ function Do_EditPrivs()
$pid = $target_group->pid();
$gid = $target_group->gid();
#
# If adding with manager privs, then make sure current user can
# bestow manager.
#
if ($trust == TBDB_TRUSTSTRING_GROUPROOT && !ISADMIN() &&
!$target_group->AccessCheck($this_user, $TB_PROJECT_BESTOWGROUPROOT)) {
SPITAJAX_ERROR(-1, "You are not allowed to bestow manager privs");
return -1;
}
# Create the user if it is not yet approved.
if ($target_user->status() == TBDB_USERSTATUS_UNAPPROVED) {
$target_user->SetStatus(TBDB_USERSTATUS_ACTIVE);
......
......@@ -189,7 +189,8 @@ $(function ()
"pid" : window.TARGET_PROJECT,
"gid" : window.TARGET_GROUP,
"canedit" : window.CANEDIT,
"canapprove" : window.CANAPPROVE}));
"canapprove" : window.CANAPPROVE,
"canbestow" : window.CANBESTOW}));
// Format dates with moment before display.
$('#members_table .format-date').each(function() {
......
......@@ -327,7 +327,8 @@ $(function ()
"pid" : window.TARGET_PROJECT,
"gid" : window.TARGET_PROJECT,
"canedit" : window.CANAPPROVE,
"canapprove" : window.CANAPPROVE}));
"canapprove" : window.CANAPPROVE,
"canbestow" : window.CANBESTOW}));
// Format dates with moment before display.
$('#members_table .format-date').each(function() {
......
......@@ -53,6 +53,8 @@ $emulablink = "$TBBASE/showgroup.php3?group=" . $group->gid_idx();
$canapprove = $group->AccessCheck($this_user, $TB_PROJECT_ADDUSER) ? 1 : 0;
$candelete = $group->AccessCheck($this_user, $TB_PROJECT_DELGROUP) ? 1 : 0;
$canedit = $group->AccessCheck($this_user, $TB_PROJECT_EDITGROUP) ? 1 : 0;
$canbestow = $group->AccessCheck($this_user,
$TB_PROJECT_BESTOWGROUPROOT) ? 1 : 0;
# Never allowed to delete project group.
if ($group->pid() == $group->gid()) {
$candelete = 0;
......@@ -68,6 +70,7 @@ echo " window.ISADMIN = $isadmin;\n";
echo " window.CANAPPROVE = $canapprove;\n";
echo " window.CANDELETE = $candelete;\n";
echo " window.CANEDIT = $canedit;\n";
echo " window.CANBESTOW = $canbestow;\n";
echo " window.EMULAB_LINK = '$emulablink';\n";
echo " window.TARGET_PROJECT = '" . $group->pid() . "';\n";
echo " window.TARGET_GROUP = '" . $group->gid() . "';\n";
......
......@@ -154,6 +154,10 @@ function Do_MemberList()
$blob["trust"] = $newTrustMap[$membership["trust"]];
$blob["approved"] = ($target_project->UserTrust($user) ==
TBDB_TRUSTSTRING_NONE ? 0 : 1);
# Can the current user delete this user from the project.
$blob["candelete"]=
(ISADMIN() ||
$target_project->CanDeleteUser($this_user, $user) ? 1 : 0);
$results[$user->uid()] = $blob;
}
......
<?php
#
# Copyright (c) 2000-2016 University of Utah and the Flux Group.
# Copyright (c) 2000-2017 University of Utah and the Flux Group.
#
# {{{EMULAB-LICENSE
#
......@@ -51,6 +51,8 @@ if (!ISADMIN() && !ISFOREIGN_ADMIN() &&
}
$emulablink = "$TBBASE/showproject.php3?project=" . $project->pid();
$canapprove = $project->AccessCheck($this_user, $TB_PROJECT_ADDUSER) ? 1 : 0;
$canbestow = $project->AccessCheck($this_user,
$TB_PROJECT_BESTOWGROUPROOT) ? 1 : 0;
echo "<link rel='stylesheet'
href='css/tablesorter.css'>\n";
......@@ -58,6 +60,7 @@ echo "<link rel='stylesheet'
echo "<script type='text/javascript'>\n";
echo " window.ISADMIN = $isadmin;\n";
echo " window.CANAPPROVE = $canapprove;\n";
echo " window.CANBESTOW = $canbestow;\n";
echo " window.EMULAB_LINK = '$emulablink';\n";
echo " window.TARGET_PROJECT = '" . $project->pid() . "';\n";
echo "</script>\n";
......
......@@ -41,12 +41,14 @@
<% if (!value.approved) { %>text-danger<% } %>">
<%- value.joined %></span>
</td>
<% if ((canedit || canapprove) && value.trust != "leader") { %>
<% if ((canedit || canapprove) &&
value.trust != "leader" && value.candelete) { %>
<% var trustvalues = ["none", "user","root","manager"]; %>
<td>
<select class="editprivs" data-uid="<%- uid %>">
<% _.each(trustvalues, function(trust) { %>
<% if (value.approved && trust == "none") { return; } %>
<% if (!canbestow && trust == "manager") { return; } %>
<option value="<%- trust %>"
<% if (value.trust == trust) { %>selected<% } %>>
<% if (trust == "none") { trust = "Please Select"; } %>
......@@ -69,7 +71,7 @@
<td><%- value.trust %></td>
<% } %>
<% if (canedit || canapprove) { %>
<% if (value.trust != "leader") { %>
<% if (value.trust != "leader" && value.candelete) { %>
<td><input class="remove-checkbox" data-uid="<%- uid %>"
type="checkbox"></td>
<% } else {%>
......@@ -114,6 +116,7 @@
<td>
<select class="editprivs" data-uid="<%- uid %>">
<% _.each(trustvalues, function(trust) { %>
<% if (!canbestow && trust == "manager") { return; } %>
<option value="<%- trust %>"
<% if (value.trust == trust) { %>selected<% } %>>
<% if (trust == "none") { trust = "Please Select"; } %>
......
......@@ -82,10 +82,11 @@ if (isset($request) && $request) {
if (isset($target_project)) {
$target_pid = $target_project->pid();
if (! $isadmin &&
! $target_project->AccessCheck($this_user, $TB_PROJECT_DELUSER)) {
USERERROR("You do not have permission to remove user ".
"$target_uid from project $target_pid!", 1);
if (! $isadmin) {
if (! $target_project->CanDeleteUser($this_user, $target_user)) {
USERERROR("You do not have permission to remove user ".
"$target_uid from project $target_pid!", 1);
}
}
$leader = $target_project->GetLeader();
......
......@@ -445,7 +445,21 @@ class Group
$mintrust = $TBDB_TRUST_GROUPROOT;
}
elseif ($access_type == $TB_PROJECT_DELUSER) {
$mintrust = $TBDB_TRUST_PROJROOT;
#
# Only project leaders can delete group root in the main
# group, group roots cannot delete each other, but that
# cannot be tested here, so we have to have a seprate
# CanDeleteUser() user function that wraps this check.
#
# In subgroups, group_root can delete each other. Hmm.
#
if ($pid != $gid) {
if (TBMinTrust(TBGrpTrust($uid, $pid, $pid),
$TBDB_TRUST_GROUPROOT)) {
return 1;
}
}
$mintrust = $TBDB_TRUST_GROUPROOT;
}
else {
TBERROR("Unexpected access type: $access_type!", 1);
......@@ -454,6 +468,16 @@ class Group
return TBMinTrust(TBGrpTrust($uid, $pid, $gid), $mintrust);
}
# Can the user delete the target user.
function CanDeleteUser($user, $target_user) {
global $TB_PROJECT_DELUSER;
if ($this->IsProjectGroup()) {
return $this->Project()->CanDeleteUser($user, $target_user);
}
return $this->AccessCheck($user, $TB_PROJECT_DELUSER);
}
#
# Return a users trust within the group.
#
......@@ -1202,10 +1226,11 @@ class Group
#
# Pretty display of group members.
#
function ShowMembers($prived = 0) {
function ShowMembers($this_user = null) {
$gid_idx = $this->gid_idx();
$pid_idx = $this->pid_idx();
$project = $this->Project();
global $TB_PROJECT_DELUSER;
$query_result =
DBQueryFatal("select uid_idx,trust from group_membership ".
......@@ -1214,7 +1239,9 @@ class Group
if (! mysql_num_rows($query_result)) {
return;
}
$showdel = (($prived && $pid_idx == $gid_idx) ? 1 : 0);
$showdel =
($pid_idx == $gid_idx && $this_user &&
$this->AccessCheck($this_user, $TB_PROJECT_DELUSER) ? 1 : 0);
$projgrp = $this->IsProjectGroup();
echo "<center>\n";
......@@ -1273,9 +1300,14 @@ class Group
echo "<td>$trust</td>\n";
if ($showdel) {
echo "<td align=center>
if ($this->CanDeleteUser($this_user, $target_user)) {
echo "<td align=center>
<a href='$deluser_url'>
<img alt='Delete User' src=redball.gif></td>\n";
}
else {
echo "<td></td>\n";
}
}
echo "</tr>\n";
}
......
......@@ -399,6 +399,21 @@ class Project
return $group->AccessCheck($user, $access_type);
}
# Can the user delete the target user.
function CanDeleteUser($user, $target_user) {
global $TB_PROJECT_DELUSER;
# Not allowed to delete yourself from a group.
if ($user->SameUser($target_user)) {
return 0;
}
if (! $this->AccessCheck($user, $TB_PROJECT_DELUSER) ||
$this->UserTrust($target_user) >= $this->UserTrust($user)) {
return 0;
}
return 1;
}
# Return the user trust within the project, which is really for the
# default group.
function UserTrust($user) {
......
<?php
#
# Copyright (c) 2000-2007 University of Utah and the Flux Group.
# Copyright (c) 2000-2017 University of Utah and the Flux Group.
#
# {{{EMULAB-LICENSE
#
......@@ -63,14 +63,6 @@ if (! ($group->AccessCheck($this_user, $TB_PROJECT_READINFO) ||
"group $gid in project $pid!", 1);
}
#
# See if user is privledged for deletion.
#
$prived = 0;
if ($isadmin || $project->AccessCheck($this_user, $TB_PROJECT_DELUSER)) {
$prived = 1;
}
#
# This menu only makes sense for people with privs to use them.
#
......@@ -99,7 +91,7 @@ if ($showmenu) {
}
$group->Show();
$group->ShowMembers($prived);
$group->ShowMembers($this_user);
if ($showmenu) {
SUBPAGEEND();
......
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