Skip to content

Commit

Permalink
Add a form to the Accounts Manager for resetting two factor authentic…
Browse files Browse the repository at this point in the history 
…ation for students.

This form does not allow the user to reset their own two factor
authentication secret, but that of other users at equal or lesser
permission level to their own.

Note that in the admin course if there are multiple admin users, then
one admin user can reset two factor authentication for another.

Also some clean up and issue fixes in the
`htdocs/js/UserList/userlist.js` file with form validation.  The
"change" event handler was being added multiple times to the users list
table. More clean up is needed though (with this and the other pages
with action forms).  There is a lot of redundancy with this
form validation implementation.
  • Loading branch information
@drgrice1
drgrice1 committed Feb 23, 2024
1 parent 39e9ecc commit 660d24b
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 60 deletions.
105 changes: 46 additions & 59 deletions htdocs/js/UserList/userlist.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,44 +25,44 @@
classlist_add_export_elements();
}

const selectUserErrMsg = document.getElementById('select_user_err_msg');
const clearSelectErrors = () => {
selectUserErrMsg?.classList.add('d-none');
for (const id of ['filter_select', 'edit_select', 'password_select', 'export_select_scope']) {
document.getElementById(id)?.classList.remove('is-invalid');
}
};

// Action form validation.
const classListTable = document.getElementById('classlist-table');
const is_user_selected = () => {
for (const user of document.getElementsByName('selected_users')) {
if (user.checked) return true;
}
document.getElementById('select_user_err_msg')?.classList.remove('d-none');
document.getElementById('classlist-table')?.addEventListener(
'change',
() => {
document.getElementById('select_user_err_msg')?.classList.add('d-none');
for (const id of ['filter_select', 'edit_select', 'password_select', 'export_select_scope']) {
document.getElementById(id)?.classList.remove('is-invalid');
}
},
{ once: true }
);
selectUserErrMsg?.classList.remove('d-none');
classListTable?.removeEventListener('change', clearSelectErrors, { once: true });
classListTable?.addEventListener('change', clearSelectErrors, { once: true });
return false;
};

document.getElementById('user-list-form')?.addEventListener('submit', (e) => {
const action = document.getElementById('current_action')?.value || '';
if (action === 'filter') {
const filter_select = document.getElementById('filter_select');
const filter = filter_select?.selectedIndex || 0;
const filter = filter_select?.value;
const filter_text = document.getElementById('filter_text');
if (filter === 1 && !is_user_selected()) {
if (filter === 'selected' && !is_user_selected()) {
e.preventDefault();
e.stopPropagation();
filter_select.classList.add('is-invalid');
filter_select.addEventListener(
'change',
() => {
document.getElementById('select_user_err_msg')?.classList.add('d-none');
document.getElementById('filter_select')?.classList.remove('is-invalid');
selectUserErrMsg?.classList.add('d-none');
filter_select.classList.remove('is-invalid');
},
{ once: true }
);
} else if (filter === 2 && filter_text.value === '') {
} else if (filter === 'match_regex' && filter_text.value === '') {
e.preventDefault();
e.stopPropagation();
document.getElementById('filter_err_msg')?.classList.remove('d-none');
Expand All @@ -76,32 +76,17 @@
{ once: true }
);
}
} else if (action === 'edit') {
const edit_select = document.getElementById('edit_select');
if (edit_select.value === 'selected' && !is_user_selected()) {
e.preventDefault();
e.stopPropagation();
edit_select.classList.add('is-invalid');
edit_select.addEventListener(
'change',
() => {
document.getElementById('select_user_err_msg')?.classList.add('d-none');
document.getElementById('edit_select')?.classList.remove('is-invalid');
},
{ once: true }
);
}
} else if (action === 'password') {
const password_select = document.getElementById('password_select');
if (password_select.value === 'selected' && !is_user_selected()) {
} else if (action === 'edit' || action === 'password') {
const action_select = document.getElementById(`${action}_select`);
if (action_select && action_select.value === 'selected' && !is_user_selected()) {
e.preventDefault();
e.stopPropagation();
password_select.classList.add('is-invalid');
password_select.addEventListener(
action_select.classList.add('is-invalid');
action_select.addEventListener(
'change',
() => {
document.getElementById('select_user_err_msg')?.classList.add('d-none');
document.getElementById('password_select')?.classList.remove('is-invalid');
selectUserErrMsg?.classList.add('d-none');
action_select.classList.remove('is-invalid');
},
{ once: true }
);
Expand All @@ -116,8 +101,8 @@
export_select.addEventListener(
'change',
() => {
document.getElementById('select_user_err_msg')?.classList.add('d-none');
document.getElementById('export_select_scope')?.classList.remove('is-invalid');
selectUserErrMsg?.classList.add('d-none');
export_select.classList.remove('is-invalid');
},
{ once: true }
);
Expand Down Expand Up @@ -149,24 +134,26 @@
{ once: true }
);
}
} else if (action === 'delete') {
const delete_confirm = document.getElementById('delete_select');
if (!is_user_selected()) {
e.preventDefault();
e.stopPropagation();
} else if (delete_confirm.value != 'yes') {
e.preventDefault();
e.stopPropagation();
document.getElementById('delete_confirm_err_msg')?.classList.remove('d-none');
delete_confirm.classList.add('is-invalid');
delete_confirm.addEventListener(
'change',
() => {
document.getElementById('delete_select')?.classList.remove('is-invalid');
document.getElementById('delete_confirm_err_msg')?.classList.add('d-none');
},
{ once: true }
);
} else if (action === 'delete' || action === 'reset_2fa') {
const action_confirm = document.getElementById(`${action}_select`);
if (action_confirm) {
if (!is_user_selected()) {
e.preventDefault();
e.stopPropagation();
} else if (action_confirm.value != 'yes') {
e.preventDefault();
e.stopPropagation();
document.getElementById(`${action}_confirm_err_msg`)?.classList.remove('d-none');
action_confirm.classList.add('is-invalid');
action_confirm.addEventListener(
'change',
() => {
action_confirm.classList.remove('is-invalid');
document.getElementById(`${action}_confirm_err_msg`)?.classList.add('d-none');
},
{ once: true }
);
}
}
}
});
Expand Down
37 changes: 36 additions & 1 deletion lib/WeBWorK/ContentGenerator/Instructor/UserList.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ use WeBWorK::Utils::Instructor qw(getCSVList);
use constant HIDE_USERS_THRESHHOLD => 200;
use constant EDIT_FORMS => [qw(save_edit cancel_edit)];
use constant PASSWORD_FORMS => [qw(save_password cancel_password)];
use constant VIEW_FORMS => [qw(filter sort edit password import export add delete)];
use constant VIEW_FORMS => [qw(filter sort edit password import export add delete reset_2fa)];

# Prepare the tab titles for translation by maketext
use constant FORM_TITLES => {
Expand All @@ -84,6 +84,7 @@ use constant FORM_TITLES => {
export => x('Export'),
add => x('Add'),
delete => x('Delete'),
reset_2fa => x('Reset Two Factor Authentication'),
save_password => x('Save Password'),
cancel_password => x('Cancel Password')
};
Expand All @@ -94,6 +95,7 @@ use constant FORM_PERMS => {
edit => 'modify_student_data',
save_password => 'change_password',
password => 'change_password',
reset2_2fa => 'change_password',
import => 'modify_student_data',
export => 'modify_classlist_files',
add => 'modify_student_data',
Expand Down Expand Up @@ -479,6 +481,39 @@ sub export_handler ($c) {
return $c->maketext('[_1] users exported to file [_2]', scalar @userIDsToExport, "$dir/$fileName");
}

sub reset_2fa_handler ($c) {
my $db = $c->db;
my $user = $c->param('user');

my $confirm = $c->param('action.reset_2fa.confirm');
my $num = 0;

return $c->maketext('Reset two factor authentication for [_1] users.', $num) unless $confirm eq 'yes';

# grep on userIsEditable would still enforce permissions, but no UI feedback
my @userIDsForReset = keys %{ $c->{selectedUserIDs} };

my @resultText;
for my $userID (@userIDsForReset) {
if ($userID eq $user) {
push @resultText, $c->maketext('You cannot reset two factor authentication for yourself!');
next;
}

unless ($c->{userIsEditable}{$userID}) {
push @resultText, $c->maketext('You are not allowed to reset two factor authenticatio for [_1].', $userID);
next;
}
my $password = $db->getPassword($userID);
$password->otp_secret('');
$db->putPassword($password);
$num++;
}

unshift @resultText, $c->maketext('Reset two factor authentication for [_1] users.', $num);
return join(' ', @resultText);
}

sub cancel_edit_handler ($c) {
if (defined $c->param('prev_visible_users')) {
$c->{visibleUserIDs} = { map { $_ => 1 } @{ $c->every_param('prev_visible_users') } };
Expand Down
1 change: 1 addition & 0 deletions templates/ContentGenerator/Instructor/UserList.html.ep
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
% my $default_choice;
%
% for my $actionID (@$formsToShow) {
% next if $actionID eq 'reset_2fa' && !$ce->two_factor_authentication_enabled;
% next if $formPerms->{$actionID} && !$authz->hasPermissions(param('user'), $formPerms->{$actionID});
%
% my $disabled = $actionID eq 'import' && !@$CSVList ? ' disabled' : '';
Expand Down
23 changes: 23 additions & 0 deletions templates/ContentGenerator/Instructor/UserList/reset_2fa_form.html.ep
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<div>
<div class="d-inline-block alert alert-danger p-1 mb-2">
<em>
<%= maketext('Warning: This will make users need to setup two factor authentication again! Only do this '
. 'for users that can no longer access the course due the account being lost in the authenticator app.')
=%>
</em>
</div>
<div class="row mb-2">
<%= label_for reset_2fa_select => maketext('Reset two factor authentication for selected users?'),
class => 'col-form-label col-form-label-sm col-auto' =%>
<div class="col-auto">
<%= select_field 'action.reset_2fa.confirm' => [
[ maketext('No') => 'no', selected => undef ],
[ maketext('Yes') => 'yes' ]
],
id => 'reset_2fa_select', class => 'form-select form-select-sm' =%>
</div>
</div>
<div id="reset_2fa_confirm_err_msg" class="alert alert-danger p-1 d-inline-flex d-none">
<%= maketext('Please confirm it is okay to reset two factor authentication for selected users.') %>
</div>
</div>
8 changes: 8 additions & 0 deletions templates/HelpFiles/InstructorUserList.html.ep
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,14 @@
. 'assignment data has been permanently deleted.') =%>
</dd>

<dt><%= maketext('Reset two factor authentication for a student in the course') %></dt>
<dd>
<%= maketext('This resets two factor authentication for a student, thus making the student need to set up two '
. 'factor authentication again. This should only be done if a student has accidentally deleted their '
. 'account or for some other reason lost their key in the authenticator app, and so can no longer access '
. 'the course. Note that this will only appear if two factor authentication is enabled for the course.') =%>
</dd>

<dt><%= maketext('Assign sets to one student') %></dt>
<dd>
<%= maketext('To assign one or more sets to an individual student click in the column "Assigned Sets" in the '
Expand Down

0 comments on commit 660d24b

Please sign in to comment.