From b23409755f3e7720d7d70ba7e0544491799c67d6 Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Fri, 31 Jul 2026 16:27:44 -0700 Subject: [PATCH 1/2] easy entry to a course from admin course Co-authored-by: Claude --- conf/defaults.config | 13 ++++ conf/localOverrides.conf.dist | 13 ++++ lib/WeBWorK/Authen.pm | 123 +++++++++++++++++++++++++++++++++- 3 files changed, 147 insertions(+), 2 deletions(-) diff --git a/conf/defaults.config b/conf/defaults.config index 8abf21ba86..ece2007b0f 100644 --- a/conf/defaults.config +++ b/conf/defaults.config @@ -688,6 +688,19 @@ $authen{proctor_module} = "WeBWorK::Authen::Proctor"; # This is used instead of $authen{user_module} when logging into the admin course. $authen{admin_module} = ['WeBWorK::Authen::Basic_TheLastOption']; +# If set to true, and if not using Saml2, Shibboleth, or CAS authentication, +# then a user who is not logged into course X, but whose browser still holds an +# active session cookie for the admin course, will be logged into course X +# automatically, provided that: +# - the site uses cookie session management (not key) +# - the admin course user holds the create_and_delete_courses permission there, and +# - course X has a user with the same user_id whose password hash matches the one in +# the admin course. Note that password hashes will match when a user is added to +# course X from the admin course at the time course X is initialized. However, +# password hashes will not match simply from setting the same password in course X +# directly. +$authen{admin_cross_course_login} = 0; + ################################################################################ # Authorization system (Make local overrides in localOverrides.conf ) ################################################################################ diff --git a/conf/localOverrides.conf.dist b/conf/localOverrides.conf.dist index e28a5aeecf..ffe3b88737 100644 --- a/conf/localOverrides.conf.dist +++ b/conf/localOverrides.conf.dist @@ -148,6 +148,19 @@ $mail{feedbackRecipients} = [ # the templates for a new course can be copied. #$modelCoursesForCopy = [ "modelCourse", "anotherModelCourse", "aThirdOne" ]; +# If set to true, and if not using Saml2, Shibboleth, or CAS authentication, +# then a user who is not logged into course X, but whose browser still holds an +# active session cookie for the admin course, will be logged into course X +# automatically, provided that: +# - the site uses cookie session management (not key) +# - the admin course user holds the create_and_delete_courses permission there, and +# - course X has a user with the same user_id whose password hash matches the one in +# the admin course. Note that password hashes will match when a user is added to +# course X from the admin course at the time course X is initialized. However, +# password hashes will not match simply from setting the same password in course X +# directly. +#$authen{admin_cross_course_login} = 1; + ################################################################################ # OpenProblemLibrary ################################################################################ diff --git a/lib/WeBWorK/Authen.pm b/lib/WeBWorK/Authen.pm index f1f72f2546..3b0a68a282 100644 --- a/lib/WeBWorK/Authen.pm +++ b/lib/WeBWorK/Authen.pm @@ -40,6 +40,8 @@ use Scalar::Util qw(weaken); use Mojo::Util qw(b64_encode b64_decode); use Math::Random::Secure qw(irand); +use WeBWorK::CourseEnvironment; +use WeBWorK::DB; use WeBWorK::Debug qw(debug); use WeBWorK::Utils qw(x runtime_use utf8Crypt cryptPassword); use WeBWorK::Utils::Logs qw(writeCourseLog); @@ -199,7 +201,9 @@ sub verify { } else { $self->write_log_entry("LOGIN FAILED $self->{log_error}") if defined $self->{log_error}; $self->maybe_kill_cookie; - $c->stash(authen_error => $self->{error}) if $self->{error} && $self->{error} =~ /\S/; + # A failed admin cross-course login attempt should not surface an error to the user. + $c->stash(authen_error => $self->{error}) + if $self->{error} && $self->{error} =~ /\S/ && ($self->{credential_source} // '') ne 'admin_cross_course'; } my $caliper_sensor = Caliper::Sensor->new($c->ce); @@ -261,6 +265,8 @@ sub do_verify { if (defined $self->{login_type} && $self->{login_type} eq 'guest') { return $self->verify_practice_user; + } elsif (($self->{credential_source} // '') eq 'admin_cross_course') { + return $self->verify_admin_cross_course_user; } else { return $self->verify_normal_user; } @@ -378,7 +384,89 @@ sub get_credentials { return 1; } - return 0; + return $self->try_admin_cross_course_credentials; +} + +=head2 try_admin_cross_course_credentials + +This is a last resort used by C when a request for this course has no cookie or +C parameter. If C<< $ce->{authen}{admin_cross_course_login} >> is enabled, the admin course +uses C session management, and the browser still holds an active session cookie +for the admin course, this decodes that cookie, confirms the session is still valid, and confirms +the admin course user has the C permission. If all of that succeeds, +C and C are set so that C can finish +authenticating the user into this course. + +=cut + +sub try_admin_cross_course_credentials { + my $self = shift; + my $c = $self->{c}; + my $ce = $c->ce; + + return 0 + unless $ce->{authen}{admin_cross_course_login} + && defined $ce->{admin_course_id} + && $ce->{courseName} ne $ce->{admin_course_id}; + + my ($ce_admin, $db_admin); + eval { + $ce_admin = WeBWorK::CourseEnvironment->new({ courseName => $ce->{admin_course_id} }); + $db_admin = WeBWorK::DB->new($ce_admin); + }; + return 0 if $@ || !$db_admin || ($ce_admin->{session_management_via} // '') ne 'session_cookie'; + + # Decode the admin course's session cookie + my $sessions = $c->app->sessions; + my $adminCookieName = 'WeBWorKCourseSession.' . $ce->{admin_course_id}; + my $cookieMethod = $sessions->encrypted ? 'encrypted_cookie' : 'signed_cookie'; + my $rawValue = $c->$cookieMethod($adminCookieName); + return 0 unless $rawValue; + $rawValue =~ y/-/=/; + my $adminSession = eval { $sessions->deserialize->(b64_decode($rawValue)) }; + return 0 unless $adminSession; + + my $expiration = $adminSession->{expiration} // $sessions->default_expiration; + my $expires = delete $adminSession->{expires}; + return 0 if !$expires && $expiration || defined $expires && $expires <= time; + + my ($adminUserID, $adminKey, $adminTimestamp) = @{$adminSession}{qw(user_id key timestamp)}; + return 0 unless $adminUserID && $adminKey; + + # Confirm the admin session is still valid + my $AdminKey = $db_admin->getKey($adminUserID); + return 0 + unless defined $AdminKey + && $AdminKey->key eq $adminKey + && time <= ($adminTimestamp // $AdminKey->timestamp) + $ce_admin->{sessionTimeout}; + + return 0 unless _admin_course_has_create_delete_permission($ce_admin, $db_admin, $adminUserID); + + my $AdminPassword = $db_admin->getPassword($adminUserID); + return 0 unless defined $AdminPassword && $AdminPassword->password =~ /\S/; + + $self->{user_id} = $adminUserID; + $self->{admin_cross_course_password} = $AdminPassword->password; + $self->{login_type} = 'normal'; + $self->{credential_source} = 'admin_cross_course'; + debug('credential source: "admin_cross_course", user: "', $self->{user_id}, '"'); + return 1; +} + +# Mirrors the relevant part of WeBWorK::Authz::hasPermissions for the admin course, since that method requires a +# full Mojolicious controller bound to the admin course, and here the controller is bound to the course being +# entered instead. +sub _admin_course_has_create_delete_permission { + my ($ce_admin, $db_admin, $user) = @_; + + my $activity_role = $ce_admin->{permissionLevels}{create_and_delete_courses}; + return 0 unless defined $activity_role && exists $ce_admin->{userRoles}{$activity_role}; + my $role_permlevel = $ce_admin->{userRoles}{$activity_role}; + + my $PermissionLevel = $db_admin->getPermissionLevel($user); + return 0 unless defined $PermissionLevel && defined $PermissionLevel->permission; + + return $PermissionLevel->permission >= $role_permlevel; } sub check_user { @@ -594,6 +682,37 @@ sub verify_normal_user { } } +=head2 verify_admin_cross_course_user + +Finishes authenticating a user identified by C. +Compares this course's password hash for the user against the admin course's hash +captured by C. + +=cut + +sub verify_admin_cross_course_user { + my $self = shift; + my $c = $self->{c}; + + my $user_id = $self->{user_id}; + + my $coursePassword = $c->db->getPassword($user_id); + unless (defined $coursePassword + && $coursePassword->password =~ /\S/ + && $coursePassword->password eq $self->{admin_cross_course_password}) + { + $self->{log_error} = 'admin cross-course login: no matching password for this user in this course'; + $self->{error} = $c->maketext(GENERIC_ERROR_MESSAGE); + return 0; + } + + return 0 unless $self->validate_user; + + $self->{session_key} = $self->create_session($user_id); + $self->{initial_login} = 1; + return 1; +} + # Returns 1 if authentication succeeded, returns 0 if required data was present but authentication failed, # and returns -1 if the password is missing. sub authenticate { From f1e033fa6eb44c531728792b7468502a5059d4ad Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Thu, 13 Aug 2026 18:06:03 -0500 Subject: [PATCH 2/2] Suggestions for admin course entry code improvements. See my review of https://github.com/openwebwork/webwork2/pull/3103 for details on the changes. --- lib/WeBWorK/Authen.pm | 44 +++++++++++++++++++------------------------ 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/lib/WeBWorK/Authen.pm b/lib/WeBWorK/Authen.pm index 3b0a68a282..51b37964fe 100644 --- a/lib/WeBWorK/Authen.pm +++ b/lib/WeBWorK/Authen.pm @@ -417,39 +417,36 @@ sub try_admin_cross_course_credentials { return 0 if $@ || !$db_admin || ($ce_admin->{session_management_via} // '') ne 'session_cookie'; # Decode the admin course's session cookie - my $sessions = $c->app->sessions; - my $adminCookieName = 'WeBWorKCourseSession.' . $ce->{admin_course_id}; - my $cookieMethod = $sessions->encrypted ? 'encrypted_cookie' : 'signed_cookie'; - my $rawValue = $c->$cookieMethod($adminCookieName); + my $sessions = $c->app->sessions; + my $rawValue = $c->signed_cookie('WeBWorKCourseSession.' . $ce->{admin_course_id}); return 0 unless $rawValue; $rawValue =~ y/-/=/; - my $adminSession = eval { $sessions->deserialize->(b64_decode($rawValue)) }; - return 0 unless $adminSession; + return 0 unless my $adminSession = $sessions->deserialize->(b64_decode($rawValue)); - my $expiration = $adminSession->{expiration} // $sessions->default_expiration; - my $expires = delete $adminSession->{expires}; - return 0 if !$expires && $expiration || defined $expires && $expires <= time; + return 0 + if !$adminSession->{expires} && ($adminSession->{expiration} // $sessions->default_expiration) + || defined $adminSession->{expires} && $adminSession->{expires} <= time; my ($adminUserID, $adminKey, $adminTimestamp) = @{$adminSession}{qw(user_id key timestamp)}; return 0 unless $adminUserID && $adminKey; # Confirm the admin session is still valid - my $AdminKey = $db_admin->getKey($adminUserID); + my $adminKeyRecord = $db_admin->getKey($adminUserID); return 0 - unless defined $AdminKey - && $AdminKey->key eq $adminKey - && time <= ($adminTimestamp // $AdminKey->timestamp) + $ce_admin->{sessionTimeout}; + unless defined $adminKeyRecord + && $adminKeyRecord->key eq $adminKey + && time <= ($adminTimestamp // $adminKeyRecord->timestamp) + $ce_admin->{sessionTimeout}; return 0 unless _admin_course_has_create_delete_permission($ce_admin, $db_admin, $adminUserID); - my $AdminPassword = $db_admin->getPassword($adminUserID); - return 0 unless defined $AdminPassword && $AdminPassword->password =~ /\S/; + my $adminPasswordRecord = $db_admin->getPassword($adminUserID); + return 0 unless defined $adminPasswordRecord && $adminPasswordRecord->password =~ /\S/; $self->{user_id} = $adminUserID; - $self->{admin_cross_course_password} = $AdminPassword->password; + $self->{admin_cross_course_password} = $adminPasswordRecord->password; $self->{login_type} = 'normal'; $self->{credential_source} = 'admin_cross_course'; - debug('credential source: "admin_cross_course", user: "', $self->{user_id}, '"'); + debug(qq{credential source: "admin_cross_course", user: "$self->{user_id}"}); return 1; } @@ -463,10 +460,10 @@ sub _admin_course_has_create_delete_permission { return 0 unless defined $activity_role && exists $ce_admin->{userRoles}{$activity_role}; my $role_permlevel = $ce_admin->{userRoles}{$activity_role}; - my $PermissionLevel = $db_admin->getPermissionLevel($user); - return 0 unless defined $PermissionLevel && defined $PermissionLevel->permission; + my $permissionLevel = $db_admin->getPermissionLevel($user); + return 0 unless defined $permissionLevel && defined $permissionLevel->permission; - return $PermissionLevel->permission >= $role_permlevel; + return $permissionLevel->permission >= $role_permlevel; } sub check_user { @@ -694,21 +691,18 @@ sub verify_admin_cross_course_user { my $self = shift; my $c = $self->{c}; - my $user_id = $self->{user_id}; - - my $coursePassword = $c->db->getPassword($user_id); + my $coursePassword = $c->db->getPassword($self->{user_id}); unless (defined $coursePassword && $coursePassword->password =~ /\S/ && $coursePassword->password eq $self->{admin_cross_course_password}) { $self->{log_error} = 'admin cross-course login: no matching password for this user in this course'; - $self->{error} = $c->maketext(GENERIC_ERROR_MESSAGE); return 0; } return 0 unless $self->validate_user; - $self->{session_key} = $self->create_session($user_id); + $self->{session_key} = $self->create_session($self->{user_id}); $self->{initial_login} = 1; return 1; }