Merge upstream WeBWorK 2.20 into UBC branch#52
Open
ltdevopsbot[bot] wants to merge 1410 commits into
Open
Conversation
…-extensions adjustments to date extension achievement items
There is really no production scenario where this should be anything but 1. If you are really serving without https, then you can change this. And good luck dealing with your users having to click through the warnings to get to your site. Note that this just works if you are using `localhost` in development also. Browsers have a built in exception for `localhost` and consider it secure without actual certificates. I am tired of seeing forum issues because system administrators forget or don't know to change this.
Change the default value of `$CookieSecure` to 1.
…-tweaks Add support for wide characters in passwords. (replacement of openwebwork#2628)
…lem-no-jquery Make JITAR disabled problems disabled without javascript.
This adds achievement items that modify the reduced scoring time period while leaving other settings alone. These are designed for classes which use reduced scoring and keep assignments open in the reduced scoring period for a long time (such as the end of the course). + ExtendReducedDate -> Extends the reduced scoring date 24 hours. + SuperExtendReducedDate -> Extends the reduced scoring date 48 hours. + NoReducedCred -> Removes the reduced scoring flag on an assignment to allow full credit after the reduced scoring date.
…levels Support for custom permission levels not in the userRoles hash (replacement of openwebwork#2629)
This way the parameters can be sorted in this one instance where this is desired.
The `formatRenderedProblem` method of the `FormatRenderedProblem` module ensures that the `debug_messages` key of the rendered problem result is an array reference, but checks the wrong thing. It checks if the result itself is an array reference instead of the `debug_messages` hash key of the result. Since the result is always a hash reference and never an array reference, this means that the debugging messages from PG are always wiped out and replaced with a reference to an empty array. This was discovered when investigating openwebwork#2661. If we want to switch the PG problem editor to using the `debug` output format of the `formatRenderedProblem` method we could, but in order to get the benefit of that change we need this.
Construct the emailable URL directly and don't use systemLink for that.
…ent-items Add achievement items that modify reduced scoring.
Add leaderboard for achievements.
Updates to grading tests for other users.
make only outer edge of achivements images transparent and fix a miss…
…red-debug-messages Fix debugging messages in the `formatRenderedProblem` method.
This is as discussed in openwebwork#2661, openwebwork#2662 and the developer meeting. The PG problem editor is switched to the debug format from the simple format. The only difference for the problem editor page is that if there are debug messages they are now shown. Usually there aren't any such messages, and so there isn't a difference. But if something is enabled like the MathObject diagnostics cmp flag, then those debug messages are shown.
This makes it so that persistence hash is only saved to the database when answers are submitted. In the case that answers can be checked by the user (via the "Check Answers" button), then the persistence hash is saved JSON encoded in a hidden form field. That case does not need the security of saving to the database, and in that case it shouldn't be saved to the database because answers aren't being recorded and the data in that case should temporary. So a hidden form field is just right for this. I objected to the previous implementation when openwebwork#1940 was submitted, but allowed it at that time, but this is how the problem_data should have been implemented. Nothing should be written to the database for this when answers are not being recorded. If an instructor is acting as a student, the previous code was saving the persistent data to the student's problem even when the instructor just enters the problem. Of course it was also saving every time the instructor did anything with the problem including using the "Preview My Answers" button, the "Check Answers" button, the "Show Correct Answers" button, the "Show Problem Grader" button. Literally any form submission of the page. That just was not thought out. The render_rpc (and html2xml) routes use the hidden form field approach to also support saving the problem data from the persistence hash. This means that problems that use the persistence hash can be tested in the PG problem editor. Note that the PERSISTENCE_HASH_UPDATE flag is removed. That didn't improve efficiency at all. The processing that was done with that was too much. Even if this were saved when it was before it would have been more efficient to just save it. The handling of the persistence hash is also reworked for PG in a corresponding pull request. PG just sends the hash, and it can contain anything that can be JSON encoded. Webwork2 just JSON encodes it and stores it, but only when answers are submitted.
Different dates can have different timezone adjustments in the case that the client timezone and server timezone are different and one of those time zones adheres to daylight savings time and the other does not. For example if the client timezone is America/Phoenix and the server timezone is America/Chicago. The America/Phoenix timezone does not adhere to daylight savings time and is always UTC-7 while America/Chicago is UTC-5 during daylight savings time, but is UTC-6 during standard time. So if an instructor is in the America/Phoenix timezone, but working on a server set for the America/Chicago timezone and selects a date that is during daylight savings time, then the current code use an incorrect differential of 1 hour because it uses the same differential for all dates. So this computes the timezone adjustment for a given date so that the correct timezone differential for that time is used. This fixes issue openwebwork#2654.
Switch the problem editor to use the debug format.
change introduction to feedback emails
Recompute timezone adjustments for a given date.
Rework problem data (or the persistence hash).
The first is CourseDBIntegrityCheck.pm that checks the database integrity and handles a database upgrade. The second is the CourseDirectoryIntegrityCheck that checks the directory structure and handles fixing that if not correct. The database and directory integrity checks do completely separate things, and directory integrity check methods should not be part of the object that is used for the database integrity check. When that object is constructed it obtains a separate database connection that is unused for the directory check and should not be obtained if that is what is being done. So now the directory check and upgrade methods are simply methods exported from the `WeBWorK::Utils::CourseDirectoryIntegrityCheck` module. The unused `lib/WeBWorK/Utils/DBUpgrade.pm` file was deleted.
…e database. This is not done by default. There are check boxes that can be selected to do this when upgrading a course. Since this process can be risky there are ample warnings recommending that an archive be made before upgrading and changing field types. Also remove the `fieldOverride` usage. There are no field overrides anymore, and the current SQL::Abstract code no longer even supports it.
For example, the checkbox label will now say "Change type of field from INT to BIGINT when upgrading" if the type of the field in the database is INT and the type in the schema is BIGINT.
…message for grouping sets. This is the message on the problem set detail instructor page that warns of a problem source being used multiple times in the same set. This message should never be shown for grouping sets. Grouping sets are allowed to be repeated. This has bugged me for a long time.
…update_97ac41b207 Updates for file lib/WeBWorK/Localize/webwork2.pot in el [Manual Sync]
…update_d23ae80e68 Updates for file lib/WeBWorK/Localize/webwork2.pot in ko [Manual Sync]
…update_9cdac29bdb Updates for file lib/WeBWorK/Localize/webwork2.pot in fr [Manual Sync]
Currently if the session has expired and one of the options in the "Format Code" tabe is used in the PG problem editor, then the message reads `Error: can't access property "status", t.result_data is undefined` which is not very informative. This makes it so that the error message in the response which is `Error: Authentication failed. Log in again to continue.` An easy way to test this is to open a problem in the problem editor, delete the cookie for the session in the developer tools, and then switch to the "Format Code" tabe and click the "Format Code" button.
…xpired-issue Fix the session expired message in the problem editor.
record/update user lis_source_did even when grade passback not in use
WeBWorK 2.20 Release Candidate
# Conflicts: # .github/workflows/linter.yml # conf/database.conf.dist
xcompass
requested changes
Jun 19, 2026
xcompass
left a comment
xcompass
left a comment
Member
There was a problem hiding this comment.
Review: REQUEST CHANGES — semantic regression in UBC's LTI/job integration
This is a vendor merge of upstream WeBWorK-2.20 (~1400 commits) into ubc. The automated checks (ancestry, no conflict markers, bash syntax) pass, but they only detect textual problems. Verified locally against merge head 92bbe13, base origin/ubc, and tag WeBWorK-2.20, the merge silently breaks UBC's LTI 1.3 and DelayedJob features.
Root cause: the conf/database.conf.dist conflict was resolved by keeping a file upstream deleted
Upstream 2.20 moved the DB layout out of conf/database.conf.dist (deleted) and into lib/WeBWorK/DB/Layout.pm. This was a modify/delete conflict, resolved by keeping UBC's database.conf.dist. But nothing includes it anymore:
- Base
ubcloaded the layout viaconf/defaults.config:include("./conf/database.conf.dist")+*dbLayout = $dbLayouts{...}(basedefaults.config:562/576). - Merged
defaults.configdropped that include, andlib/WeBWorK/DB.pm:161now builds the layout withdatabaseLayout($ce->{courseName})(upstream'sWeBWorK::DB::Layout).
So conf/database.conf.dist is now orphaned dead config, and the active layout = upstream's Layout.pm.
CRITICAL — UBC LTI 1.3 (AGS/NRPS, Canvas, Moodle) crashes at runtime
Layout.pm registers only lti_launch_data + lti_course_map. UBC's LTI tables — lti_resource_link, lti_user, lti_contexts, lti_nonces, lti_access_tokens (WeBWorK::DB::Record::LTI1p3::*) — exist only in the orphaned file, so DB.pm never creates handles for them. UBC's accessor methods survived the merge (DB.pm:2113 getLTIContext, 2279 getLTIResourceLink, 2335 getLTIUser) and dereference $self->{lti_contexts} / {lti_resource_link} handles that are never instantiated. The launch path calls them:
lib/LTI1p3/Entrypoint/Launch.pm:104,237→$db->getLTIContext(...)lib/LTI1p3/Entrypoint/Launch.pm:269,271→$db->getLTIResourceLink/newLTIResourceLink(...)
The app boots, but an LTI launch dereferences an undefined table handle → runtime crash.
HIGH — UBC DelayedJob background jobs break the same way
funcmap, job, note, error, exitstatus (WeBWorK::DB::Record::DelayedJob::*) also live only in the orphaned file (upstream uses Minion). UBC's job code is exercised: lib/DelayedJob/Service.pm dispatches PushUserGrades / PushClassGrades / GetClassMembership / SendCaliperEvent, invoked from lib/WeBWorK/Utils/ProblemProcessing.pm:224 and lib/Caliper/Sensor.pm:59. Grade push-back and Caliper events will fail.
HIGH — No Perl/runtime/test validation was run
The PR notes Perl checks couldn't run (missing Mojo::Base). A merge this size, with a modify/delete resolution, must boot in the Docker/CI env and pass the test suite plus a real LTI launch before merge.
MEDIUM
- Two parallel LTI implementations now coexist (UBC
LTI1p3::*vs upstreamLTILaunchData/LTICourseMap). Decide which is authoritative for UBC. .github/workflows/linter.ymlconflict kept UBC's older config (perl:5.34,Perl::Tidy@20220613,branches-ignore: ubc) — intentional, but the PR isn't linted to 2.20 standards. Please confirm.
Required before merge
- Decide LTI strategy (adopt upstream 2.20 LTI vs. keep UBC's LTI1p3) and reconcile the layout in
lib/WeBWorK/DB/Layout.pmaccordingly — register UBC's tables there, or port the AGS/NRPS code onto upstream's schema. - Decide job strategy (Minion vs. DelayedJob) and register/remove tables to match.
- Remove or properly re-wire the orphaned
conf/database.conf.dist. - Boot a test instance and run the suite + a real LTI launch (Canvas and Moodle) before merging.
Note: the bot's checks can't catch any of this — there's no textual conflict because UBC's and upstream's LTI live in differently-named files, so both were kept silently. Consider adding a "boots + tests pass + LTI smoke test" gate to the merge bot.
Upstream 2.20 moved the database layout from conf/database.conf.dist into lib/WeBWorK/DB/Layout.pm and deleted database.conf.dist. The 2.20 merge kept UBC's database.conf.dist, but the merged defaults.config no longer includes it and DB.pm now builds the layout from databaseLayout(). As a result UBC's LTI Advantage and DelayedJob tables were never registered, so LTI launches (getLTIContext/getLTIResourceLink) and DelayedJob grade sync would dereference undefined table handles and crash at runtime. Re-register UBC's tables in databaseLayout(), translated to the 2.20 entry shape, alongside upstream's lti_launch_data/lti_course_map (different tables): - lti_resource_link, lti_user, lti_contexts, lti_nonces, lti_access_tokens - funcmap, job, note, error, exitstatus Remove the now-orphaned conf/database.conf.dist and fix the stale reference to it in conf/site.conf.dist.
Member
Pushed a fix for the LTI/DelayedJob DB-layer regression (7c7e14f)This addresses the CRITICAL + HIGH findings from my review. Strategy chosen: keep UBC's LTI 1.3 authoritative (minimal change), leaving upstream's What the commit does:
Validation done:
Still required before merge (the validation HIGH finding):
Not in scope here: migrating to / removing upstream's LTI stack. The two stacks coexist (different tables and URLs); consolidating them is a separate follow-up if desired. |
…validation
Two regressions the 2.20 merge introduced with no textual conflict, so they
passed the automated checks:
1. verify(): the merge kept UBC's use of $log_error but dropped its
`my $log_error = $self->{log_error};` declaration. Under `use strict` the
undeclared $log_error is a compile-time fatal, so WeBWorK::Authen fails to
load and all authentication breaks. Restore the declaration.
2. killSession(): the merge dropped upstream 2.20's
`delete $c->stash->{'webwork2.database_session'};`. In 2.20, store_session()
(run in after_dispatch) re-persists the session key unless that stash entry
is cleared, so logout was re-saving the session instead of invalidating it.
Restore the cleanup so logout deletes the server-side session.
Verified with a strict-compile check: the pre-fix verify() fails with
"Global symbol \$log_error requires explicit package name"; both subs compile
cleanly after the fix.
Member
Pushed fixes for two CRITICAL merge regressions (2881efa)Both were introduced by the 2.20 merge with no textual conflict, so they passed the automated checks. 1. Proof (strict-compile check of the merged file before the fix): 2. Both subs now compile cleanly under Still open from the second-pass review (not addressed here)
Strong recommendationAdd |
…n config
Four critical regressions from the upstream 2.20 merge - all merged
cleanly with no textual conflict, none could boot or run:
- conf/defaults.config: drop the dead $ConfigValues block and its fatal
include('conf/LTIConfigValues.config'). 2.20 deleted that file (config
values now come from lib/WeBWorK/ConfigValues.pm), so the include made
every CourseEnvironment->new die and nothing could boot.
- conf/defaults.config: rename $sessionKeyTimeout to $sessionTimeout
(the 2.20 name; Authen.pm reads sessionTimeout with no fallback, so
undef made every session expire one second after creation).
- Pass $ce to WeBWorK::DB->new at 12 call sites (LTI 1.3 entrypoints,
SAML2 ACS controller, DelayedJob workers, cron scripts). 2.20 removed
$ce->{dbLayout} and DB->new now takes the course environment; passing
the old key crashed every LTI/SAML2 login and all grade sync.
- Retarget WeBWorK::Utils imports after the 2.20 Utils split
(Utils::DateTime/Files/JITAR/Sets) in 7 files that croaked at compile
time; swap formatDateTime args for the new (time, format, timezone)
order; export UBC's 5-return grade_set variant from WeBWorK::Utils
(with its moved jitar/after helpers imported); point StudentProgress
at Utils::Sets for list_set_versions (Utils::Grades was deleted);
drop unused grade_*/readFile/runtime_use imports.
- Replace merge-mangled WeBWorK::Authen::Shibboleth (unclosed sub that
failed perltidy CI; neither parent's logic survived intact) with the
pre-merge UBC version, updated for the sessionTimeout rename.
Claude-Session: https://claude.ai/code/session_01TFNbNsmYakWLrkzqeWNY3C
Member
Review round 3: consistency of UBC changes × upstream 2.20 — 6 criticals found, fixed in c55b203Focus: do the recent Verified clean first
CRITICAL — all fixed in c55b203
Still open (not fixed here)
RecommendationThe fixes above are static-analysis-verified (stubbed |
- setmaker.js: set the alert toast title/message via textContent instead of interpolating into innerHTML (js/xss-through-dom, high). All callers pass plain text, so rendering is unchanged. - linter.yml, main.yml: limit the default GITHUB_TOKEN to contents: read (actions/missing-workflow-permissions). Neither workflow uses the token beyond checkout; Docker Hub pushes use dedicated secrets. Claude-Session: https://claude.ai/code/session_01TFNbNsmYakWLrkzqeWNY3C
One-time formatting pass with the CI-pinned Perl::Tidy version so the lint workflow inherited from upstream 2.20 passes. The ubc branch was deliberately exempted from this check (branches-ignore), so UBC files were never tidied; the PR-triggered run enforces it regardless. Formatting only - verified no semantic change (import/export cross-check, syntax checks, and spot checks of the earlier fix sites all pass). Claude-Session: https://claude.ai/code/session_01TFNbNsmYakWLrkzqeWNY3C
Perl::Tidy 20220613 does not converge in one pass on the nested reply->exception(maketext(...)) chain in Launch.pm and a boolean assignment in AssignmentAndGradeService.pm; CI runs against the once-tidied tree and found the second-pass delta. Verified stable under a further pass. Claude-Session: https://claude.ai/code/session_01TFNbNsmYakWLrkzqeWNY3C
Member
All CI checks now pass (b626507)Follow-ups since the round-3 fixes (c55b203):
Checks: perltidy ✅ CodeQL ✅ build ✅ Analyze ✅ Still open before merge (from the round-3 review): the two HIGHs — |
…oding
Two HIGH findings from the round-3 review:
- Restore bin/generate-OPL-set-def-lists.pl (byte-identical to the
pre-merge ubc version). Upstream 2.20 deleted it in favor of prebuilt
metadata-release artifacts, but docker-entrypoint.sh still invokes it
when the set-def JSON files are missing - under `set -eo pipefail` a
fresh-volume deploy exited 127 and the container aborted. Restoring
the script keeps startup self-contained (local generation, no network
or database dependency at that point in the entrypoint); all of its
dependencies (OPLUtils::writeJSONtoFile, $problemLibrary{root},
$contribLibrary{root}) still exist in 2.20.
- lib/Caliper/Sensor.pm: the merge replaced `use JSON` with Mojo::JSON
but left two `JSON->new->canonical->encode` calls, which would die on
the first Caliper event. Use JSON::PP (Perl core) instead - same
canonical (sorted-key) semantics, and unlike JSON.pm it is guaranteed
present now that 2.20 dropped the json packages from the docker build.
The Mojo::JSON encode_json import was otherwise unused.
Claude-Session: https://claude.ai/code/session_01TFNbNsmYakWLrkzqeWNY3C
Member
Both remaining HIGHs fixed (0209dd0) — CI still fully green
Checks on 0209dd0: perltidy ✅ CodeQL ✅ build ✅ Analyze ✅ Everything from the round-3 review is now closed except the documented mediums (dead |
Found by a runtime smoke test of the built image (boot the container,
construct CourseEnvironment, load the touched modules, assemble the full
Mojolicious app) — none of these are visible to perl -c / perltidy / CodeQL.
- Dockerfile-prod: add three dependencies that 2.20 code loads
unconditionally but the UBC-custom prod Dockerfile (no upstream
counterpart, so the merge never synced it) was missing. The dev
Dockerfile/DockerfileStage1 already install them.
* libmime-base32-perl — 2.20's WeBWorK::Utils::TOTP (two-factor auth)
does `use MIME::Base32`, and WeBWorK::Authen loads TOTP
unconditionally (Authen.pm:45). Without it Authen.pm fails to
compile, so *all* authentication and the app itself are dead on the
production image. CRITICAL, and it is the exact CI-built artifact.
* libgd-barcode-perl — TwoFactorAuthentication.pm does
`use GD::Barcode::QRcode`; 2FA is wired into the login flow.
* Archive::Zip::SimpleZip (cpanm) — FileManager.pm does
`use Archive::Zip::SimpleZip`; the instructor File Manager page
fails to load without it.
- ProblemSetList.pm: restore DEFAULT_VISIBILITY_STATE and
DEFAULT_ENABLED_REDUCED_SCORING_STATE. The merge kept UBC's pre-2.20
local set-def import (which references them) but took upstream's
constant block, which dropped them — so the Sets Manager page failed to
compile ("Bareword ... not allowed").
- Utils.pm: drop the redundant `use WeBWorK::Utils::JITAR` import added in
c55b203. jitar_id_to_seq and jitar_problem_adjusted_status are already
defined locally in this file, so the import only produced "Subroutine
redefined" warnings on every load. `after` (from Utils::DateTime) is the
only helper that actually had to be imported.
Runtime smoke test result: CourseEnvironment constructs, WeBWorK::DB->new($ce)
connects with the UBC LTI/DelayedJob tables registered, every touched module
loads, and the full app assembles all 97 routes (LTI 1.3, LTI Advantage,
SAML2) — all green.
Claude-Session: https://claude.ai/code/session_01TFNbNsmYakWLrkzqeWNY3C
Member
Runtime smoke test — booted the CI-built image; found + fixed a new CRITICAL (fd333fe)Static checks (CI, It validated every prior fix at runtime ✅
…and it caught a new CRITICAL that no static check can seeThe production image cannot load 2.20 introduced
The same commit also restores two constants dropped from After the fix the smoke test is fully green: Non-issues confirmed (checked, no action)
One advisory (not changed)
What this does and doesn't coverThe smoke test proves the app boots and assembles every route with a real DB, and that all the merge-touched modules load. It does not exercise a live Shibboleth/SAML2 login or a real LTI 1.3 launch/grade-sync — those need an external IdP and LMS. Those routes now exist and their handler modules load; a staging run against a real IdP/Canvas is still the final gate before production. |
The prod image pinned PG_BRANCH=PG-2.18+ while webwork2 is 2.20. PG and
webwork2 release in lockstep, and 2.20 code depends on PG 2.20 APIs that
2.18 doesn't have:
- SampleProblemParser exports getSampleProblemCode in 2.20 (2.18 only has
parseSampleProblem/generateMetadata) -> PGProblemEditor.pm fails to
compile ("getSampleProblemCode is not exported"), so the PG Problem
Editor page dies with template errors ($problemContents/$formsToShow/
$actionFormTitles never stashed).
- Plots::Plot/Axes/Tikz/JSXGraph/GD were reorganized in PG 2.20; 2.18
lacks Plots/Plot.pm -> "Failed to evaluate module Plots::*" at startup.
- WeBWorK::PG::Localize is new in PG 2.20 -> "Can't locate" at startup.
Found by exercising the running 2.20 image (PG Problem Editor) in staging.
Claude-Session: https://claude.ai/code/session_01TFNbNsmYakWLrkzqeWNY3C
Brings up the full WeBWorK 2.20 stack (app, MariaDB, Rserve, the two Minion/DelayedJob workers, and maildev) from a locally built image: docker compose -f docker-compose.test.yml up --build -d # http://localhost:8080/webwork2/admin (admin / admin) - Builds the app once from Dockerfile-prod (tagged webwork-local:pr52) and reuses it for the workers; no image pull. - Forces $CookieSecure=0 (with a trailing `1;` so localOverrides.conf still returns true) so login works over plain http://localhost. - Sensible local defaults for DB, secret, SMTP (maildev), and timezone; OPL/htdocs data persisted in named volumes so restarts don't re-clone. Claude-Session: https://claude.ai/code/session_01TFNbNsmYakWLrkzqeWNY3C
xcompass
approved these changes
Jul 10, 2026
… plugin
The 2.20 merge left two parallel SAML2 stacks wired on overlapping
/saml2/* paths: UBC's custom Mojolicious::Plugin::Saml2 (a thin
Authen::Saml2 that delegates to a plugin router, YAML-configured) and
upstream 2.20's self-contained Authen::Saml2 + ContentGenerator::Saml2
(perl-conf-configured), which the merge pulled in via Utils/Routes.pm.
Upstream's implementation is a strict superset of UBC's -- same
Net::SAML2 stack, plus TOTP two-factor integration, SP-initiated single
logout, multiple IdPs, DB-backed replay protection, disk-cached IdP
metadata, and a fixed NameID fallback.
This adopts upstream's stack and removes UBC's plugin:
- lib/WeBWorK/Authen/Saml2.pm: replaced UBC's thin delegating module
with upstream 2.20's full implementation (byte-identical to the
WeBWorK-2.20 tag).
- Deleted lib/Mojolicious/Plugin/Saml2/ (7 files).
- Removed the plugin-load block in lib/Mojolicious/WeBWorK.pm.
- Deleted conf/authen_saml2.dist.yml (YAML template); upstream's
conf/authen_saml2.conf.dist is now the config template.
- Kept (already present from the merge, now the single live stack):
ContentGenerator/Saml2.pm and the saml2_* routes in Utils/Routes.pm.
DEPLOYMENT / OPS ACTION REQUIRED -- the config source changes:
- Stop shipping conf/authen_saml2.yml (YAML). Provide
conf/authen_saml2.conf (perl) built from authen_saml2.conf.dist and
include it from localOverrides.conf.
- Config moves into the $ce->{saml2} tree; map the old YAML keys:
idp.metadata_url -> $saml2{idps}{<name>} + $saml2{active_idp}
sp.entity_id -> $saml2{sp}{entity_id}
sp.attributes -> $saml2{sp}{attributes} (attribute OIDs)
bypass_query -> $saml2{bypass_query}
sp.cert / sp.signing_key (inline YAML) -> $saml2{sp}{certificate_file}
and $saml2{sp}{private_key_file} (FILE PATHS --
cert/key are now read from files, not inline)
- Routes are now fixed at /saml2/{metadata,acs,error,logout} (no longer
configurable via the plugin's route block).
- Newly available (opt-in): $saml2{sp}{enable_sp_initiated_logout},
$saml2{twoFAOnlyWithBypass}, and multiple $saml2{idps} entries.
Validated on the CI-built branch image (deps present: Net::SAML2 0.85,
URN::OASIS::SAML2): all three changed modules compile; the app boots with
no plugin and no errors; Authen::Saml2 loads and is the active
user_module; the four upstream saml2 routes register (97 total, no
duplicate plugin routes); and the SP metadata endpoint returns valid
signed SAML metadata. The live IdP assertion round-trip is
upstream-2.20-identical code and is validated against the real IdP at
deploy.
Claude-Session: https://claude.ai/code/session_01TFNbNsmYakWLrkzqeWNY3C
…pstream formatting
UBC's linter pinned Perl::Tidy 20220613 (Jun 2022) and enforces it
tree-wide (`perltidy ... && git diff --exit-code`), but upstream WeBWorK
2.20 is formatted with 20240903. The two versions format differently --
e.g. 20220613 writes `sub foo ($sig)` (space before the signature paren),
20240903 writes `sub foo($sig)` -- so the 2.20 merge's perltidy pass had
to reformat 22 upstream files just to pass CI. That created gratuitous
diffs against upstream that would recur on every future merge.
Bump the pin to 20240903 (the version upstream 2.20 itself uses in
check-formats.yml) everywhere it appears -- .github/workflows/linter.yml
and the three Dockerfiles -- and re-tidy the tree under it:
- Pure-vendored upstream files revert to upstream's exact formatting
and are now byte-identical to the WeBWorK-2.20 tag (HalfCreditProb,
HalfCreditSet, ResurrectGW, ResurrectHW, LTIAdvantage, ConfigValues,
Hardcopy, HardcopyRenderedProblem; Authen/Saml2 already matched).
- UBC-authored files that format differently between the two versions
are re-tidied to 20240903 (StudentProgress.pm, UserList.pm).
No code changes: every touched .pm/.pl is content-identical to the prior
commit modulo whitespace; the only non-formatting changes are the four
version-pin bumps. The tree is idempotent under 20240903 (a second
perltidy pass is a no-op), so the linter gate stays green. Future upstream
merges no longer reformat vendored files.
Claude-Session: https://claude.ai/code/session_01TFNbNsmYakWLrkzqeWNY3C
Replace UBC's custom perltidy-only linter.yml with upstream 2.20's
check-formats.yml so the format-checking workflow stays in sync with
upstream. Kept verbatim except two adaptations (noted in the file):
- a least-privilege `permissions: contents: read` block (CodeQL, matches
UBC's main.yml);
- UBC's default branch `ubc` added to the push branches-ignore list.
This also brings in upstream's prettier job (JS/CSS/SCSS/HTML + *.dist.yml),
which UBC's linter lacked. Ran prettier over the three UBC-authored files
that weren't prettier-clean so the new gate passes -- formatting only, no
logic change (prettier is AST-preserving; gateway.js was verified with
`node --check`):
- htdocs/js/GatewayQuiz/gateway.js (had mixed tab/space indentation)
- htdocs/themes/math4/_theme-overrides.scss
- docker-config/docker-compose.dist.yml
The perltidy job is unchanged in effect (Perl::Tidy 20240903; the tree is
already idempotent from the previous commit). The three Dockerfile
Perl::Tidy pins remain at 20240903.
Claude-Session: https://claude.ai/code/session_01TFNbNsmYakWLrkzqeWNY3C
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
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.
Merges upstream openwebwork/webwork2 tag WeBWorK-2.20 into the UBC branch.
Verification performed locally:
Notes: