Skip to content

easy entry to a course from admin course - #3103

Open
Alex-Jordan wants to merge 1 commit into
openwebwork:developfrom
Alex-Jordan:admin-entry
Open

easy entry to a course from admin course#3103
Alex-Jordan wants to merge 1 commit into
openwebwork:developfrom
Alex-Jordan:admin-entry

Conversation

@Alex-Jordan

Copy link
Copy Markdown
Contributor

This is marked draft. Even though I'm targeting WeBWorK-2.21 right now, that is only so that the diff is clearly visible in GitHub. Later this will be re-targeted to develop, following the 2.21 release.

This (optional feature, off by default) makes it so that if you are using cookies for session management (not keys) and if you have a valid active session in the admin course, then that will smoothly grant you access into any other course. Some conditions are needed, of course:

  • There is a user in that other course with the same username as your admin course user.
  • That admin course user has to have high level permissions (create_and_delete_courses).
  • The password hash is the same for both users (the one in the admin course and the one in the other course you intend to enter).

The main feature here (from my perspective) is that you can click links in the admin course and just be granted a session in the course you clicked on. This even works if that other course only allows users to enter through an LMS. You can also just click any link to any course, like say one in a student help email, and gain a session cookie. And you won't need to type a password.

All of this still requires 2FA for the course you are entering, assuming 2FA is enabled for that course, for a user of your level. That's actually something I would prefer not to have to do if I'm already authenticated in the admin course. But that could be changed later if this PR is not too objectionable.

Technical note: just because your user in the admin course and user in some other course have the same password, they would still have different password hashes if passwords were set independently. This really only works if the user in the other course were added to that other course as an admin user at the time the other course was initialized.

@Alex-Jordan
Alex-Jordan marked this pull request as draft July 31, 2026 23:50
@Alex-Jordan
Alex-Jordan changed the base branch from WeBWorK-2.21 to develop August 4, 2026 21:21
@Alex-Jordan

Copy link
Copy Markdown
Contributor Author

This is now retargeted to develop. However at the moment, develop has not yet been updated with the WeBWorK-2.21/main branch.

@drgrice1
drgrice1 force-pushed the develop branch 2 times, most recently from 914c17f to a7d9a03 Compare August 4, 2026 21:37
@drgrice1

drgrice1 commented Aug 4, 2026

Copy link
Copy Markdown
Member

Develop is now up to date. So you can rebase onto it now.

Co-authored-by: Claude <noreply@anthropic.com>
@Alex-Jordan

Copy link
Copy Markdown
Contributor Author

Rebased and pushed.

@drgrice1 drgrice1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are just code suggestions. I am still working on analyzing the effect of this on other authentication modules.

I have created a pull request to this branch with these suggested code changes.

Comment thread lib/WeBWorK/Authen.pm

use WeBWorK::CourseEnvironment;
use WeBWorK::DB;
use WeBWorK::Debug;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be

Suggested change
use WeBWorK::Debug;
use WeBWorK::Debug qw(debug);

and is the reason for the conflict.

Comment thread lib/WeBWorK/Authen.pm
Comment on lines +420 to +423
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);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be

Suggested change
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});

Webwork uses signed_cookies (the default). So there is not need to check if the session cookie is encrypted or not. It isn't. Also the encrypted cookie feature was not added to Mojolicious until version 9.39, and we currently allow version 9.34 or newer (except a few bad versions) of Mojolicious. Even if we do move to requiring newer versions of Mojolicious and use encrypted cookies, this check would not be necessary. This would then use the encrypted_cookie method instead. We know which one we are using, so no need to check.

Comment thread lib/WeBWorK/Authen.pm
Comment on lines +426 to +427
my $adminSession = eval { $sessions->deserialize->(b64_decode($rawValue)) };
return 0 unless $adminSession;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need for the eval here. Mojolicious does not use an eval in their code for this, so I see no reason that we should. In fact, might as well use essentially their code here and make this

Suggested change
my $adminSession = eval { $sessions->deserialize->(b64_decode($rawValue)) };
return 0 unless $adminSession;
return 0 unless my $adminSession = $sessions->deserialize->(b64_decode($rawValue));

Comment thread lib/WeBWorK/Authen.pm
return 0 unless $adminUserID && $adminKey;

# Confirm the admin session is still valid
my $AdminKey = $db_admin->getKey($adminUserID);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not use Pascal case for variable names. Use camel case. I have been working on removing that sort of thing in the webwork2 code. I recommend using the variable $adminKeyRecord here, since $adminKey is already used above, and that makes the difference clear here. The actual key in the database is the key column, and this is the database record that contains that column.

Comment thread lib/WeBWorK/Authen.pm

return 0 unless _admin_course_has_create_delete_permission($ce_admin, $db_admin, $adminUserID);

my $AdminPassword = $db_admin->getPassword($adminUserID);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also should not be Pascal case. I recommend using $adminPasswordRecord for consistency with the $adminKeyRecord variable above.

Comment thread lib/WeBWorK/Authen.pm
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);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize this variable was copied from Authz.pm, but this also should not be Pascal case. The Authz.pm file needs a lot of clean up. It is a mess.

Comment thread lib/WeBWorK/Authen.pm
my $self = shift;
my $c = $self->{c};

my $user_id = $self->{user_id};

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need for a local $user_id variable. Just use $self->{user_id} in the two places the $user_id variable is used in this method.

Comment thread lib/WeBWorK/Authen.pm
&& $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);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete this line. The only case in which this method is called is if the credential_source is 'admin_cross_course', and in that case $self->{error} is also ignored (see line 206). So there is no point in setting it.

Comment thread lib/WeBWorK/Authen.pm
Comment on lines +429 to +431
my $expiration = $adminSession->{expiration} // $sessions->default_expiration;
my $expires = delete $adminSession->{expires};
return 0 if !$expires && $expiration || defined $expires && $expires <= time;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot to mention this. I included it in the pull request.

Most of this code seems to be copied from the load method of the Mojolicious::Sessions package. They delete the expires key from the session, but this shouldn't. This could be cleaned up and changed to

Suggested change
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;

@drgrice1

Copy link
Copy Markdown
Member

So as far as the other authentication modules go, here is my assessment.

  • This is compatible with LDAP authentication. That is assuming the admin course user has a password set in the admin course and that is copied to other courses.
  • This is not compatible with Saml2 authentication, but probably could be made to work with it.
  • This is not compatible with Shibboleth authentication. It might be possible to make it compatible with that module, but this would be challenging to say the least.
  • I didn't really test it, but this will work fine with the LTI authentication modules since they completely fall back to the basic authentication module in the cases of interest anyway.
  • I don't have a way to test CAS authentication, but that probably doesn't matter. It is broken anyway.

So probably for now, you could just add comments in the documentation stating that this does not work with Saml2 and Shibbolith. Perhaps later, if there is demand, this could be extended to work for those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants