Skip to content

Commit 84e1e17

Browse files
committed
Protect against a user in the admin course changing or resetting their own OTP secret.
1 parent 6e0bcec commit 84e1e17

File tree

1 file changed

+35
-13
lines changed

1 file changed

+35
-13
lines changed

lib/WeBWorK/ContentGenerator/CourseAdmin.pm

+35-13
Original file line numberDiff line numberDiff line change
@@ -2452,11 +2452,15 @@ sub manage_otp_secrets_form ($c) {
24522452
$courses->{$courseID} = [ $dbs->{$courseID}->listUsers ];
24532453
}
24542454

2455-
# Process the confirmed rest or copy actions here.
2455+
# Process the confirmed reset or copy actions here.
24562456
if ($c->param('otp_confirm_reset')) {
24572457
my $total = 0;
24582458
my $courseID = $c->param('sourceResetCourseID');
24592459
for my $user ($c->param('otp_reset_row')) {
2460+
if ($courseID eq $c->ce->{courseName} && $user eq $c->param('user')) {
2461+
$c->addbadmessage($c->maketext('You may not reset your own OTP secret!'));
2462+
next;
2463+
}
24602464
my $password = $dbs->{$courseID}->getPassword($user);
24612465
if ($password && $password->otp_secret) {
24622466
$password->otp_secret('');
@@ -2473,6 +2477,11 @@ sub manage_otp_secrets_form ($c) {
24732477
my $total = 0;
24742478
for my $row ($c->param('otp_copy_row')) {
24752479
my ($s_course, $s_user, $d_course, $d_user) = split(':', $row);
2480+
if ($d_course eq $c->ce->{courseName} && $d_user eq $c->param('user')) {
2481+
$c->addbadmessage(
2482+
$c->maketext('You cannot overwrite your OTP secret with one from another course or user!'));
2483+
next;
2484+
}
24762485
my $s_password = $dbs->{$s_course}->getPassword($s_user);
24772486
if ($s_password && $s_password->otp_secret) {
24782487
# Password may not be defined if using external auth, so create new password record if not.
@@ -2576,17 +2585,24 @@ sub copy_otp_secrets_confirm ($c) {
25762585
$dbs{$d_course} = WeBWorK::DB->new($dest_ce->{dbLayouts}{ $dest_ce->{dbLayoutName} });
25772586
}
25782587

2579-
my $d_user_password = $dbs{$d_course}->getPassword($d_user);
2580-
if (!defined $d_user_password) {
2581-
# Just because there is no password record, the user could still exist when using external auth.
2582-
unless ($dbs{$d_course}->existsUser($d_user)) {
2583-
$dest_error = 'warning';
2584-
$error_message = $c->maketext('User does not exist - Skipping');
2585-
$skip = 1;
2588+
if ($d_course eq $c->ce->{courseName} && $d_user eq $c->param('user')) {
2589+
$dest_error = 'danger';
2590+
$error_message =
2591+
$c->maketext('You cannot overwrite your OTP secret with one from another course or user!');
2592+
$skip = 1;
2593+
} else {
2594+
my $d_user_password = $dbs{$d_course}->getPassword($d_user);
2595+
if (!defined $d_user_password) {
2596+
# Just because there is no password record, the user could still exist when using external auth.
2597+
unless ($dbs{$d_course}->existsUser($d_user)) {
2598+
$dest_error = 'warning';
2599+
$error_message = $c->maketext('User does not exist - Skipping');
2600+
$skip = 1;
2601+
}
2602+
} elsif ($d_user_password->otp_secret) {
2603+
$dest_error = 'danger';
2604+
$error_message = $c->maketext('OTP Secret is not empty - Overwritting');
25862605
}
2587-
} elsif ($d_user_password->otp_secret) {
2588-
$dest_error = 'danger';
2589-
$error_message = $c->maketext('OTP Secret is not empty - Overwritting');
25902606
}
25912607

25922608
push(
@@ -2625,8 +2641,14 @@ sub reset_otp_secrets_confirm ($c) {
26252641
my $db = WeBWorK::DB->new($ce->{dbLayouts}{ $ce->{dbLayoutName} });
26262642
my @rows;
26272643
for my $user (@dest_users) {
2628-
my $password = $db->getPassword($user);
2629-
my $error = $password && $password->otp_secret ? '' : $c->maketext('OTP Secret is empty - Skipping');
2644+
my $error = '';
2645+
2646+
if ($source_course eq $c->ce->{courseName} && $user eq $c->param('user')) {
2647+
$error = $c->maketext('You may not reset your own OTP secret!');
2648+
} else {
2649+
my $password = $db->getPassword($user);
2650+
$error = $c->maketext('OTP Secret is empty - Skipping') unless $password && $password->otp_secret;
2651+
}
26302652

26312653
push(
26322654
@rows,

0 commit comments

Comments
 (0)