diff --git a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm index 84a4c4f114..12d6f21708 100644 --- a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm +++ b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm @@ -2427,30 +2427,25 @@ sub do_save_lti_course_map ($c) { # Form to copy or reset OTP secrets. sub manage_otp_secrets_form ($c) { - my $courses = {}; - my $dbs = {}; - my $skipped_courses = []; - my $show_all_courses = $c->param('show_all_courses') || 0; + my $courses = {}; + my $dbs = {}; # Create course data first, since it is used in all cases and initializes course db references. for my $courseID (listCourses($c->ce)) { my $ce = WeBWorK::CourseEnvironment->new({ courseName => $courseID }); - $dbs->{$courseID} = WeBWorK::DB->new($ce->{dbLayouts}{ $ce->{dbLayoutName} }); - - # By default ignore courses larger than 200 users, as this can cause a large load building menus. - my @users = $dbs->{$courseID}->listUsers; - if ($show_all_courses || scalar @users < 200) { - $courses->{$courseID} = \@users; - } else { - push(@$skipped_courses, $courseID); - } + $dbs->{$courseID} = WeBWorK::DB->new($ce->{dbLayouts}{ $ce->{dbLayoutName} }); + $courses->{$courseID} = [ $dbs->{$courseID}->listUsers ]; } - # Process the confirmed rest or copy actions here. + # Process the confirmed reset or copy actions here. if ($c->param('otp_confirm_reset')) { my $total = 0; my $courseID = $c->param('sourceResetCourseID'); for my $user ($c->param('otp_reset_row')) { + if ($courseID eq $c->ce->{courseName} && $user eq $c->param('user')) { + $c->addbadmessage($c->maketext('You may not reset your own OTP secret!')); + next; + } my $password = $dbs->{$courseID}->getPassword($user); if ($password && $password->otp_secret) { $password->otp_secret(''); @@ -2467,6 +2462,11 @@ sub manage_otp_secrets_form ($c) { my $total = 0; for my $row ($c->param('otp_copy_row')) { my ($s_course, $s_user, $d_course, $d_user) = split(':', $row); + if ($d_course eq $c->ce->{courseName} && $d_user eq $c->param('user')) { + $c->addbadmessage( + $c->maketext('You cannot overwrite your OTP secret with one from another course or user!')); + next; + } my $s_password = $dbs->{$s_course}->getPassword($s_user); if ($s_password && $s_password->otp_secret) { # Password may not be defined if using external auth, so create new password record if not. @@ -2485,11 +2485,7 @@ sub manage_otp_secrets_form ($c) { } } - return $c->include( - 'ContentGenerator/CourseAdmin/manage_otp_secrets_form', - courses => $courses, - skipped_courses => $skipped_courses - ); + return $c->include('ContentGenerator/CourseAdmin/manage_otp_secrets_form', courses => $courses); } # Deals with both single and multiple copy confirmation. @@ -2574,17 +2570,24 @@ sub copy_otp_secrets_confirm ($c) { $dbs{$d_course} = WeBWorK::DB->new($dest_ce->{dbLayouts}{ $dest_ce->{dbLayoutName} }); } - my $d_user_password = $dbs{$d_course}->getPassword($d_user); - if (!defined $d_user_password) { - # Just because there is no password record, the user could still exist when using external auth. - unless ($dbs{$d_course}->existsUser($d_user)) { - $dest_error = 'warning'; - $error_message = $c->maketext('User does not exist - Skipping'); - $skip = 1; + if ($d_course eq $c->ce->{courseName} && $d_user eq $c->param('user')) { + $dest_error = 'danger'; + $error_message = + $c->maketext('You cannot overwrite your OTP secret with one from another course or user!'); + $skip = 1; + } else { + my $d_user_password = $dbs{$d_course}->getPassword($d_user); + if (!defined $d_user_password) { + # Just because there is no password record, the user could still exist when using external auth. + unless ($dbs{$d_course}->existsUser($d_user)) { + $dest_error = 'warning'; + $error_message = $c->maketext('User does not exist - Skipping'); + $skip = 1; + } + } elsif ($d_user_password->otp_secret) { + $dest_error = 'danger'; + $error_message = $c->maketext('OTP Secret is not empty - Overwritting'); } - } elsif ($d_user_password->otp_secret) { - $dest_error = 'danger'; - $error_message = $c->maketext('OTP Secret is not empty - Overwritting'); } push( @@ -2623,8 +2626,14 @@ sub reset_otp_secrets_confirm ($c) { my $db = WeBWorK::DB->new($ce->{dbLayouts}{ $ce->{dbLayoutName} }); my @rows; for my $user (@dest_users) { - my $password = $db->getPassword($user); - my $error = $password && $password->otp_secret ? '' : $c->maketext('OTP Secret is empty - Skipping'); + my $error = ''; + + if ($source_course eq $c->ce->{courseName} && $user eq $c->param('user')) { + $error = $c->maketext('You may not reset your own OTP secret!'); + } else { + my $password = $db->getPassword($user); + $error = $c->maketext('OTP Secret is empty - Skipping') unless $password && $password->otp_secret; + } push( @rows, diff --git a/templates/ContentGenerator/CourseAdmin/copy_otp_secrets_confirm.html.ep b/templates/ContentGenerator/CourseAdmin/copy_otp_secrets_confirm.html.ep index 81c8a3d3d9..4b72b3ee3b 100644 --- a/templates/ContentGenerator/CourseAdmin/copy_otp_secrets_confirm.html.ep +++ b/templates/ContentGenerator/CourseAdmin/copy_otp_secrets_confirm.html.ep @@ -40,7 +40,7 @@ - <%= $c->hidden_fields('subDisplay', 'show_all_courses') =%> + <%= $c->hidden_fields('subDisplay') =%> % if ($total > 0) { % my $skipped = @$action_rows - $total; <%= content 'hidden-rows' %> diff --git a/templates/ContentGenerator/CourseAdmin/manage_otp_secrets_form.html.ep b/templates/ContentGenerator/CourseAdmin/manage_otp_secrets_form.html.ep index 6d10a86335..e84af83c11 100644 --- a/templates/ContentGenerator/CourseAdmin/manage_otp_secrets_form.html.ep +++ b/templates/ContentGenerator/CourseAdmin/manage_otp_secrets_form.html.ep @@ -8,17 +8,6 @@ %

<%= maketext('Manage OTP Secrets') %> <%= $c->helpMacro('AdminManageOTPSecrets') %>

<%= form_for current_route, method => 'POST', begin =%> - % if (@$skipped_courses) { -
-
- <%= maketext('The following large courses are not shown: [_1]', join(', ', @$skipped_courses)); =%> -
-

- <%= submit_button maketext('Show All Courses'), - name => 'show_all_courses', class => 'btn btn-primary' =%> -

-
- % }
- <%= $c->hidden_fields('subDisplay', 'show_all_courses') =%> + <%= $c->hidden_fields('subDisplay') =%> <%= hidden_field action => $active_tab, id => 'current_action' =%>
<%= submit_button $active_tab eq 'single' diff --git a/templates/ContentGenerator/CourseAdmin/reset_otp_secrets_confirm.html.ep b/templates/ContentGenerator/CourseAdmin/reset_otp_secrets_confirm.html.ep index d804518ee5..7e936e6216 100644 --- a/templates/ContentGenerator/CourseAdmin/reset_otp_secrets_confirm.html.ep +++ b/templates/ContentGenerator/CourseAdmin/reset_otp_secrets_confirm.html.ep @@ -28,7 +28,7 @@
- <%= $c->hidden_fields('subDisplay', 'show_all_courses', 'sourceResetCourseID') =%> + <%= $c->hidden_fields('subDisplay', 'sourceResetCourseID') =%> % if ($total > 0) { % my $skipped = @$action_rows - $total; <%= content 'hidden-rows' %> diff --git a/templates/HelpFiles/AdminManageOTPSecrets.html.ep b/templates/HelpFiles/AdminManageOTPSecrets.html.ep index 7db4f8c18c..16c54b7ef0 100644 --- a/templates/HelpFiles/AdminManageOTPSecrets.html.ep +++ b/templates/HelpFiles/AdminManageOTPSecrets.html.ep @@ -19,8 +19,3 @@ <%= maketext('If resetting secrets, choose a source course ID, then choose one (or more) users whose secrets ' . 'will be reset. Note, the user(s) will have to setup two factor authentication afterwards.') =%>

-

- <%= maketext('By default, courses larger than 200 users will not be shown, as this can cause extra CPU usage ' - . 'when generating the user menus. If any large course is skipped, a message will inform you of which ' - . 'courses were skipped, and a button to show all courses can be used to include those courses.') =%> -