Skip to content

Commit d81c523

Browse files
committed
Protect against a user in the admin course changing or resetting their own OTP secret.
1 parent b795782 commit d81c523

File tree

1 file changed

+35
-13
lines changed

1 file changed

+35
-13
lines changed

lib/WeBWorK/ContentGenerator/CourseAdmin.pm

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2413,11 +2413,15 @@ sub manage_otp_secrets_form ($c) {
24132413
$courses->{$courseID} = [ $dbs->{$courseID}->listUsers ];
24142414
}
24152415

2416-
# Process the confirmed rest or copy actions here.
2416+
# Process the confirmed reset or copy actions here.
24172417
if ($c->param('otp_confirm_reset')) {
24182418
my $total = 0;
24192419
my $courseID = $c->param('sourceResetCourseID');
24202420
for my $user ($c->param('otp_reset_row')) {
2421+
if ($courseID eq $c->ce->{courseName} && $user eq $c->param('user')) {
2422+
$c->addbadmessage($c->maketext('You may not reset your own OTP secret!'));
2423+
next;
2424+
}
24212425
my $password = $dbs->{$courseID}->getPassword($user);
24222426
if ($password && $password->otp_secret) {
24232427
$password->otp_secret('');
@@ -2434,6 +2438,11 @@ sub manage_otp_secrets_form ($c) {
24342438
my $total = 0;
24352439
for my $row ($c->param('otp_copy_row')) {
24362440
my ($s_course, $s_user, $d_course, $d_user) = split(':', $row);
2441+
if ($d_course eq $c->ce->{courseName} && $d_user eq $c->param('user')) {
2442+
$c->addbadmessage(
2443+
$c->maketext('You cannot overwrite your OTP secret with one from another course or user!'));
2444+
next;
2445+
}
24372446
my $s_password = $dbs->{$s_course}->getPassword($s_user);
24382447
if ($s_password && $s_password->otp_secret) {
24392448
# Password may not be defined if using external auth, so create new password record if not.
@@ -2537,17 +2546,24 @@ sub copy_otp_secrets_confirm ($c) {
25372546
$dbs{$d_course} = WeBWorK::DB->new($dest_ce);
25382547
}
25392548

2540-
my $d_user_password = $dbs{$d_course}->getPassword($d_user);
2541-
if (!defined $d_user_password) {
2542-
# Just because there is no password record, the user could still exist when using external auth.
2543-
unless ($dbs{$d_course}->existsUser($d_user)) {
2544-
$dest_error = 'warning';
2545-
$error_message = $c->maketext('User does not exist - Skipping');
2546-
$skip = 1;
2549+
if ($d_course eq $c->ce->{courseName} && $d_user eq $c->param('user')) {
2550+
$dest_error = 'danger';
2551+
$error_message =
2552+
$c->maketext('You cannot overwrite your OTP secret with one from another course or user!');
2553+
$skip = 1;
2554+
} else {
2555+
my $d_user_password = $dbs{$d_course}->getPassword($d_user);
2556+
if (!defined $d_user_password) {
2557+
# Just because there is no password record, the user could still exist when using external auth.
2558+
unless ($dbs{$d_course}->existsUser($d_user)) {
2559+
$dest_error = 'warning';
2560+
$error_message = $c->maketext('User does not exist - Skipping');
2561+
$skip = 1;
2562+
}
2563+
} elsif ($d_user_password->otp_secret) {
2564+
$dest_error = 'danger';
2565+
$error_message = $c->maketext('OTP Secret is not empty - Overwritting');
25472566
}
2548-
} elsif ($d_user_password->otp_secret) {
2549-
$dest_error = 'danger';
2550-
$error_message = $c->maketext('OTP Secret is not empty - Overwritting');
25512567
}
25522568

25532569
push(
@@ -2586,8 +2602,14 @@ sub reset_otp_secrets_confirm ($c) {
25862602
my $db = WeBWorK::DB->new($ce);
25872603
my @rows;
25882604
for my $user (@dest_users) {
2589-
my $password = $db->getPassword($user);
2590-
my $error = $password && $password->otp_secret ? '' : $c->maketext('OTP Secret is empty - Skipping');
2605+
my $error = '';
2606+
2607+
if ($source_course eq $c->ce->{courseName} && $user eq $c->param('user')) {
2608+
$error = $c->maketext('You may not reset your own OTP secret!');
2609+
} else {
2610+
my $password = $db->getPassword($user);
2611+
$error = $c->maketext('OTP Secret is empty - Skipping') unless $password && $password->otp_secret;
2612+
}
25912613

25922614
push(
25932615
@rows,

0 commit comments

Comments
 (0)