Skip to content

Commit

Permalink
prevent editing of elevated users
Browse files Browse the repository at this point in the history
prevent password changes
align login-status right
lint
  • Loading branch information
drdrew42 committed Feb 10, 2023
1 parent 90f34b9 commit 159e292
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 15 deletions.
2 changes: 1 addition & 1 deletion htdocs/themes/math4/system.html.ep
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
<div id="masthead" class="row" role="banner">
<div class="col-md-2 webwork_logo"><%= $c->webwork_logo %></div>
<div class="col-md-6 institution_logo"><%= $c->institution_logo %></div>
<div id="login-status" class="col-md-4"><%= include 'ContentGenerator/Base/login_status' %></div>
<div id="login-status" class="col-md-4 text-end"><%= include 'ContentGenerator/Base/login_status' %></div>
</div>
% # Breadcrumb
<div id="breadcrumb-row" class="row my-2">
Expand Down
25 changes: 19 additions & 6 deletions lib/WeBWorK/ContentGenerator/Instructor/UserList.pm
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,9 @@ sub pre_header_initialize ($c) {

return unless $authz->hasPermissions($user, 'access_instructor_tools');

$c->{editMode} = $c->param('editMode') || 0;
$c->{passwordMode} = $c->param('passwordMode') || 0;
$c->{userPermission} = $db->getPermissionLevel($user)->permission;
$c->{editMode} = $c->param('editMode') || 0;
$c->{passwordMode} = $c->param('passwordMode') || 0;

return if ($c->{passwordMode} || $c->{editMode}) && !$authz->hasPermissions($user, 'modify_student_data');

Expand Down Expand Up @@ -375,6 +376,7 @@ sub delete_handler ($c) {
my $db = $c->db;
my $user = $c->param('user');
my $scope = $c->param('action.delete.scope');
my $perm = $c->{userPermission};

my @userIDsToDelete = ();
if ($scope eq 'selected') {
Expand All @@ -389,7 +391,13 @@ sub delete_handler ($c) {
my $num = 0;
foreach my $userID (@userIDsToDelete) {
if ($user eq $userID) { # don't delete yourself!!
$error = $c->maketext('You cannot delete yourself!');
$error .= $c->maketext(' You cannot delete yourself!');
next;
}

my ($thisUser) = List::Util::first { $userID eq $_->{user_id} } @{ $c->{visibleUsers} };
if ($thisUser->{permission} > $perm) {
$error .= $c->maketext(' You cannot delete someone with permissions higher than your own.');
next;
}
delete $allUserIDs{$userID};
Expand All @@ -403,7 +411,7 @@ sub delete_handler ($c) {
$c->{visibleUserIDs} = [ keys %visibleUserIDs ];
$c->{selectedUserIDs} = [ keys %selectedUserIDs ];

return $c->maketext('Deleted [_1] users.', $num) . ($error ? " $error" : '');
return $c->maketext('Deleted [_1] users.', $num) . ($error ? "$error" : '');
}

sub add_handler ($c) {
Expand Down Expand Up @@ -493,15 +501,17 @@ sub cancel_edit_handler ($c) {

sub save_edit_handler ($c) {
my $db = $c->db;
my $editorUser = $c->param('user');
my $editorUserPermission = $db->getPermissionLevel($editorUser)->permission;
my $editorUserPermission = $c->{userPermission};

my @visibleUserIDs = @{ $c->{visibleUserIDs} };
foreach my $userID (@visibleUserIDs) {
my $User = $db->getUser($userID);
die $c->maketext('record for visible user [_1] not found', $userID) unless $User;
my $PermissionLevel = $db->getPermissionLevel($userID);
die $c->maketext('permissions for [_1] not defined', $userID) unless defined $PermissionLevel;
# delete requests for elevated users should never make it this far
die $c->maketext('insufficient permission to edit [_1]', $userID)
unless ($editorUserPermission >= $PermissionLevel);
foreach my $field ($User->NONKEYFIELDS()) {
my $param = "user.$userID.$field";
if (defined $c->param($param)) {
Expand Down Expand Up @@ -547,6 +557,9 @@ sub save_password_handler ($c) {
foreach my $userID (@visibleUserIDs) {
my $User = $db->getUser($userID);
die $c->maketext('record for visible user [_1] not found', $userID) unless $User;
# password requests for elevated users should never make it this far
die $c->maketext('insufficient permission to edit [_1]', $userID)
unless ($c->{userPermission} >= $User->permission);
my $param = "user.${userID}.new_password";
if ($c->param($param)) {
my $newP = $c->param($param);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
<tbody class="table-group-divider">
% my %selectedUserIDs = map { $_ => 1 } @{ $c->{selectedUserIDs} };
% for (@{ $c->{visibleUsers} }) {
% next if ( $c->{editMode} && $_->{permission} > $c->{userPermission});
<%= include 'ContentGenerator/Instructor/UserList/user_row',
user => $_, userSelected => exists $selectedUserIDs{ $_->user_id } =%>
% }
Expand Down
26 changes: 18 additions & 8 deletions templates/ContentGenerator/Instructor/UserList/user_row.html.ep
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
% my $statusClass = $ce->status_abbrev_to_name($user->status);
% my $allowed = $c->{userPermission} >= $user->{permission} ? 1 : 0;
%
<tr>
% unless ($c->{editMode} || $c->{passwordMode}) {
% # Select checkboxes
<td>
<%= check_box selected_users => $user->user_id, id => $user->user_id . '_checkbox',
class => 'form-check-input', $userSelected ? (checked => undef) : () =%>
% if ($allowed) {
<%= check_box selected_users => $user->user_id, id => $user->user_id . '_checkbox',
class => 'form-check-input', $userSelected ? (checked => undef) : () =%>
% }
</td>
% # User id
<td>
Expand All @@ -21,12 +24,14 @@
) =%>
% }
<% end =%>
<%= link_to $c->systemLink(url_for, params => { editMode => 1, visible_users => $user->user_id }),
begin =%>
<i class="icon fas fa-pencil-alt" aria-hidden="true"
data-alt="<%= maketext('Link to Edit Page for [_1]', $user->user_id) %>">
</i>
% end
% if ($allowed) {
<%= link_to $c->systemLink(url_for, params => { editMode => 1, visible_users => $user->user_id }),
begin =%>
<i class="icon fas fa-pencil-alt" aria-hidden="true"
data-alt="<%= maketext('Link to Edit Page for [_1]', $user->user_id) %>">
</i>
% end
% }
</div>
</td>
% # Login Status
Expand All @@ -53,6 +58,11 @@
<div class="alert alert-danger px-1 py-0 m-0">
<%= maketext('You may not change your own password here!') =%>
</div>
% } elsif (!$allowed) {
% # Prevent modification of users with elevated permissions.
<div class="alert alert-danger px-1 py-0 m-0">
<%= maketext('You may not change this user\'s password!') =%>
</div>
% } else {
% my $fieldName = 'user.' . $user->user_id . '.new_password';
% param($fieldName, undef);
Expand Down

0 comments on commit 159e292

Please sign in to comment.