WeBWorK 2.19 Release Candidate #2416
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `Imager::QRCode` seems to have some issues on various linux distributions. The Ubuntu packages apparently have some changes applied that fix these issues. However, if the package is installed from cpan it fails. On Oracle it seems the same issues occur. In testing the `GD::Barcode::QRcode` package works on Ubuntu, Oracle, and Redhat.
Roles with this permission level are required to use two factor authentication to sign in. Users below this permission level can sign in directly. The default user role with this permission is "student". But if there is strong opposition to this default, then I suppose it could be switched to "login_proctor". Note that even if this is set to "guest", guest users will still be able to sign in without two factor authentication since it never really makes sense to have guests (i.e. practice users) use two factor authentication.
Apparently Microsoft Authenticator does not support the SHA256 or SHA512 algorithms.
This makes two factor authentication work more reliably for DUO.
The setting in defaults.config which can be overidden in
localOverrides.conf is $twoFA{skip_verification_code_interval}. The
default value is set to one year.
Doing this forces the `GD::Barcode::QRcode` package to auto detect the version, ans works on all platforms to generate a working QR code. However, on some platforms (well really all of them except the packages in the Ubuntu repositories) this emits warnings. To fix that, the warnings are simply disabled when the image is generated.
Fill in empty dates with valid dates for date picker groups.
Add two factor authentication.
fix aria-controls in instructor tools index page
Set handling methods are now in WeBWorK::Utils::Sets. This module
exports the methods:
* format_set_name_internal
* format_set_name_display
* grade_set
* grade_gateway
* grade_all_sets
* is_restricted
* get_test_problem_position
* list_set_versions
Note that `list_set_version` was the only method in
WeBWorK::Utils::Grades which is now deleted.
JITAR problem methods are in WeBWorK::Utils::JITAR. This module exports
the methods:
* seq_to_jitar_id
* jitar_id_to_seq
* is_jitar_problem_hidden
* is_jitar_problem_closed
* jitar_problem_finished
* jitar_problem_adjusted_status
* prob_id_sort
File system interaction methods are in WeBWorK::Utils::Files. This
module exports the methods:
* surePathToFile
* readFile
* readDirectory
* listFilesRecursive
* path_is_subdir
Note that the methods createDirectory, makeTempDirectory,
removeTempDirectory that were in WeBWorK::Utils have been deleted, since
they are not used. The `readFile` method has been updated to use
`slurp` from `Mojo::File`. The `surePathToFile` method now uses
`make_path` from `Mojo::File`.
A bug in the `listFilesRecursive` method was fixed that prevented it
from recursing into symbolic links to directories. Furthermore, the
method was completely rewritten to use File::Find which should be faster
than the recursive method, and is certainly cleaner codewise.
The `templates/ContentGenerator/Instructor/FileManager/delete.html.ep`
file has been switched from using the the `readDirectory` and
`listFilesRecursive` methods to using the `list` and `list_tree` methods
of `Mojo::File`. Now that `listFilesRecursive` correctly recurses into
symbolic links, this method reports the incorrect number of files being
deleted in directories containing symbolic links. The actual deletion
does not follow symbolic links, so the files inside symbolic links
should not be counted. The `list_tree` method does not follow symbolic
links.
With the above change, the only place that `listFilesRecursive` is
actually used is in `templates/ContentGenerator/Instructor/ProblemSetDetail.pm`
to list set and hardcopy headers. Since `listFilesRecursive` is fixed,
you will now see set headers that are inside symbolic links in a courses
templates directory. Note that this does NOT include set headers that
are in any library listed in `$courseFiles{problibs}` (Library, Contrib,
and capaLibrary by default) since those directories are pruned. That is
true regardless of if those are directories or symbolic links.
Logging methods are provided by WeBWorK::Utils::Logs. This module
exports the methods:
* writeCourseLog
* writeLog
* writeTimingLogEntry
Note that there is no longer a method `writeCourseLogGivenTime`. Instead
the `writeCourseLog` method accepts an optional $time argument which
defaults to the current time. To be able to do that the accepted
`$message` argument can not be a slurpy parameter. That was never
actually used anyway though (and would not have worked well if it was).
Date/Time handling methods are provided by WeBWorK::Utils::DateTime.
This module exports the methods:
* before
* between
* after
* formatDateTime
* getDefaultSetDueDate
* parseDateTime
* timeToSec
The constituency_hash, dequote, fisher_yates_shuffle, list2hash,
pretty_print_rh, and ref2string methods in WeBWorK::Utils were deleted
since they are not used.
The rest of the WeBWorK::Utils module is cleaned up. Signatures are now
used in that file, as well as the new files.
POD has been added for all methods in these files.
Also there is some inclusion clean up.
Also add `follow_skip => 2` to the `File::Find` `find` call in the
`WeBWorK::Utils::Instructor` `getDefList` method. If two symbolic links
point to the same location, then `find` will die when it is about to
process a file pointed to by the second link because it has already
processed that file. It shouldn't die. Setting `follow_skip => 2` makes
`find` ignore the duplicate. This was discovered while fixing the
`listFilesRecursive` method.
The `readDirectory` method is removed. The `read_dir` and `getCSVList` methods of `WeBWorK::Utils::Instructor` which used `readDirectory` have also been removed, and `Mojo::File::list` used directly where those methods were used previously. The `get_library_sets` and `get_library_pgs` methods of `WeBWorK::ContentGenerator::Instructor::SetMaker` have been renamed to `get_course_pg_dirs` and `get_pg_files_in_dir` to more accurately reflect what they really do. They have also been heavily rewritten to effectively utilize the `Mojo::File::list` method and the features of `Mojo::File` and `Mojo::Collection`. Note that issues were seen with those methods that were fixed. Namely there were cases where pg files in a courses templates directory could not be accessed from the library browser as they should be. Rather than updating several of the unused methods in `WebworkWebservice::LibraryActions`, those methods have been removed. The methods that remain are the only ones used by webwork2.
…he others. That is it uses methods from the webwork2 perl `lib` but still can be executed from anywhere. This means the `cryptPassword` method in `WeBWorK::Utils` is only in one place, and does not need to be kept in sync with a copy of it in this script.
The methods from that file are now in lib/WeBWorK/DB/Utils.pm. The three methods from lib/WeBWorK/Utils/Task.pm are closely related to some of the methods already in lib/WeBWorK/DB/Utils.pm, and so they just fit in there. Note that the `fake_user` method was deleted, since it is not used anywhere. They aren't "tasks" in any case.
The session `flash` method is similar to the `session` method added previously, and is a method of the WeBWorK::Authen object attached to the controller. It uses the `flash` method of `Mojolicious::Plugin::DefaultHelpers` if `session_management_via` is "session_cookie" and imitates that with the database session otherwise. This method saves data to the session that will persist only for the next request. This is then used to save `status_messages` when redirects occur. This fixes issue #2336, since the `status_message` URL parameter is no longer used. We need to make sure that we never again use a URL parameter to pass HTML.
…e.pl script) The `download-OPL-metadata-release.pl` is extracting the downloaded metadata release to the wrong directory, and so the rest of the script fails. It fails quietly, and doesn't actually restore any tables. So a die statement was added to cover this case.
Fix the OPL-update script (or really the download-OPL-metadata-release.pl script)
Add a session flash and use it to pass status messages for redirects.
Separate Utils.pm into smaller Utils/*.pm modules.
This is essentially what @Alex-Jordan suggested in #2335 (see #2335 (comment)).
Change the label on the "Skip two factor" check box.
add a div for the score summary in rpc default format
The value attribute of the hidden key input does not have a value attribute. Need to stop at the element in the first step.
Fix the instructor_rpc endpoint usage for the library browser.
…bmissions. Any action on the instructor index page (the Instructor Tools) that possibly includes multiple users or multiple sets can not be a GET request (i.e., a redirect). They must be a POST request (i.e., a form submission). This is because GET requests limit the amount of data that they can contain 2-8 KB (depending on the browser and server). If a large number of users or sets are selected this can easily exceed this size limit. The "show answers" action makes this even worse by including the problem ids. POST requests have much larger limits depending on both the browser and the server. Mojolicious limits this to 16GB by default, and it varies with the browser. In any case it is quite unlikely that this limit will be attained for any of these requests. This issue came up with this page previously when all requests were redirects (GET requests). There was an issue reported several years ago about this when an instructor selected all users in a large class. So I fixed this by using a direct form submission via the formaction atttibutes on the respective submit buttons for which this was a problem. In some cases, since the parameters don't always match up with the page the form will be submitted to, javascript fills in the required parameters. At the time I did not fix the "edit set for users" action (i.e., "Edit one set's details for some or all users"). It was more challenging than the others, since it needs to post to a different URL depending on which set is selected. I have a little more experience now, and figured out how to do it this time by using a regular expression replacement to add the correct set to the URL (and preserve URL parameters like effectiveUser and such -- Although I think these aren't actually needed since they are in the form). The "show answers" action (i.e., "View answer log for selected users, for selected sets") is new, and I missed this when it was added. Although, that one is even more challenging, because it needs to have the problems ids of the selected sets. Those problem ids are now added to the page in hidden inputs that the javascript can access to fill in the needed information. I also added the `formtarget => 'WW_Info'` in this case. That means that the answer log will open in a new page like it usually does. I don't exaclty like this though, but if this is not done, and then the instructor clicks the "Display Past Answers" button on the answer log page then another window is opened with a second instance of the answer log. Perhaps it is time to stop opening pages in new windows like this? This is considered bad practice in many circles.
add mathInteraction.pg to orientation
…k-space Replace narrow no-break spaces in dates with regular spaces for PG.
when grade passback only happens on mass update intervals, let studen…
Prevent protected files from being modified by unprivileged users.
case insensitive sorting by name
Note that the imagemagick-allow-pdf-read.patch is no longer needed. Ubuntu 24.04 allows PDF read rights by default. The reason this restriction was previously applied was due to a security vulnerability in ghostscript. That vulnerability has long been fixed (even for Ubuntu 22.04 although the policy was not updated). The patch is left in the repository for Ubuntu 22.04 users. Also, the Mojo::SQLite version downgrade is not needed anymore with the version of Mojo::SQLite in Ubuntu 24.04.
I always forget to do this one.
These were identified in a scan of @glarose's server. The first cross-site scripting vulnerability occurs because an invalid set id is found and when that invalid set id is reported it is done so without XML escaping it. To test it use (for example): `https://your.server.edu/webwork2/courseID/feedback%3cimg%20src%3da%20onerror%3dalert(1)%3e` Of course replace `https://your.server.edu/webwork2/courseID` with the URL on your server for a valid course. The second vulnerability occurs because a problem id that starts with an integer and has certain text appended can validate by only the integer part, and yet be used in the page including that text part. To test this use `https://your.server.edu/webwork2/courseID/setID/1%3cimg%20src%3da%20onerror%3dalert(1)%3e` where 1 is a valid problem in the set. To fix this the problemID route parameter is forced to be an integer. So if you use a URL like the above, it will now just give "Page not found". The last vulnerability is probably more prevalent in the code than just what is fixed in this pull request. It occurs on the user options page when certain URL params are added. For example, `https://your.server.edu/webwork2/courseID/options?useMathQuill=%3cscript%3ealert(1)%3c%2fscript%3&changeOptions=1`. Although the issue was specifically reported for the `useMathQuill` parameter, it occurs with any of the integer valued option parameters on that page. Note that the `changeOptions` parameter is needed in all cases, because it only occurs when saving options. The problem is that there is an error saving those values to the database because they are not integer values. Then we put the exception in `$@` unescaped into the html, and that exception message includes the original invalid value. To fix this the value of `$@` is no longer used in the error message. Instead it is logged to the app log file (via $c->log->error), and a generic error is reported to the user. This needs to be done throughout the code. We really never should use a direct system exception message in html. That should be logged for the system administrator, and a generic error message shown in the UI. The same goes for Perl warnings in general. So there is much work to be done here.
Adds LTIGradeOnSubmit, LTIMassUpdateInterval, LTI{v1p1}{BasicConsumerSecret}
to the list of variables that can be configured by a course instructor.
Used to keep the system (or default) LTI keys hidden from users when configuring them via the instructor course configuration page. This is done by replacing any output with a SECRET_STRING, set to '*****', that is displayed to the user. Currently this option only works for 'text' configuration objects.
update usage of checkName
Update the version in the README.md file.
…lnerabilities Fix some cross-site scripting vulnerabilities.
also clear tmpEdit folder when 'cleaning' a newly unarchived course
achievement to grant extensions
let certain achievements apply to closed sets
add problem_grader permission
Update the docker build to Ubuntu 24.04.
Add more configurable LTI variables to config page.
Add username and password option for SMTP authentication
pstaabp
approved these changes
Aug 6, 2024
Alex-Jordan
approved these changes
Aug 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As usual, target pull request to this branch if you want it to go into the release.