Development: Add LTI 1.3 interop test coverage and harden Lti13LaunchFilter exception handling - #12778
Merged
Conversation
Both classes escaped the filter as raw RuntimeExceptions because DispatcherServlet's HandlerExceptionResolver does not run for exceptions thrown inside servlet filters. The leak surfaced two ways: 1. JwtException (BadJwtException, JwtValidationException) from NimbusJwtDecoder during Step 3b signature/expiry validation — every real LMS that sends an invalid token would hit this. Now mapped to 500 alongside OAuth2AuthenticationException. 2. HttpStatusException (BadRequestAlertException et al.) from lti13Service.performLaunch — e.g. "Course not found" was misleadingly bubbling up as an uncaught RuntimeException instead of the 400 the exception itself carries. Now mapped to its embedded status code so clients see the correct response. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the test gaps that allowed #12739 (Spring Boot 4 broke UriComponentsBuilder.fromHttpUrl) to ship undetected: Per-PR (server-tests, ~32s incremental): - Lti13Step3JwtValidationIntegrationTest: signs an id_token with a local RSA key, serves the matching JWKS over an in-process JDK HttpServer, and POSTs to /auth-login. Exercises the full Spring Security filter chain through NimbusJwtDecoder.withJwkSetUri() with no mocks. Three tests cover the happy path, wrong-key rejection, and expired-token rejection. Nightly (new workflow, ~60s wall clock): - NightlyLtiMoodleInteropTest: boots bitnamilegacy/moodle:5.0.2 + postgres via Testcontainers, extracts Moodle's RSA private key from mdl_config_plugins, and verifies every step of the LTI 1.3 OIDC launch end-to-end: - Step 1 (initiate-login) builds the auth-request URL Moodle expects - Step 3a (redirect proxy) accepts a real Moodle-signed JWT - Step 3b (auth-login) fetches Moodle's JWKS over real HTTP and validates a JWT signed by Moodle's actual lti_sign_jwt() PHP function (invoked via docker exec on a script copied into the container) - Full success path with a Course + OnlineCourseConfiguration + TextExercise fixture — performLaunch auto-creates the user and joins them to the course's student group - Disabled Canvas + edX stubs reserve matrix slots in the workflow. - .github/workflows/nightly-lti-interop.yml: cron 03:00 UTC, matrix over (moodle, canvas, edx), Slack failure alert via LTI_NIGHTLY_SLACK_WEBHOOK. - gradle/test.gradle: exclude `nightly-lti` tag from the default per-PR run; opt in with -DincludeTags='nightly-lti'. Design notes in documentation/docs/developer/lti-nightly-interop-plan.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
tests
server
krusche
changed the title
General: Add LTI 1.3 interop test coverage and harden Lti13LaunchFilter exception handlingDevelopment: Add LTI 1.3 interop test coverage and harden Lti13LaunchFilter exception handling
Member
Author
|
@Claudia-Anthropica review |
Contributor
|
@krusche on it! Taking a look now 👀 |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds in-process JWKS-based test coverage for LTI 1.3 Step 3 JWT validation, a nightly real-Moodle Testcontainers interop suite (with disabled Canvas/edX placeholders), and hardens Lti13LaunchFilter to catch JwtException and HttpStatusException so signature failures and BadRequestAlertExceptions from Lti13Service.performLaunch surface with the correct HTTP status instead of leaking past DispatcherServlet's exception resolver.
Changes:
- New
Lti13Step3JwtValidationIntegrationTestexercising the full Spring Security filter chain against a JDK-hosted JWKS endpoint with real RSA-signed tokens. - New
nightly-lti-tagged Moodle Testcontainers suite (with stub Canvas/edX classes), a cron-triggered GitHub Actions workflow, a Gradle exclusion for the tag in the default run, and a PHP signing helper invoked viadocker exec. Lti13LaunchFilternow catchesHttpStatusException(status forwarded) andJwtException(mapped to 500) in addition to the existing exception types.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/de/tum/cit/aet/artemis/lti/config/Lti13LaunchFilter.java | Adds HttpStatusException outer catch and JwtException inner catch so JWT/LTI errors map to proper HTTP statuses. |
| src/test/java/de/tum/cit/aet/artemis/lti/Lti13Step3JwtValidationIntegrationTest.java | Per-PR Step 3b coverage with in-process JWKS server and RSA-signed tokens. |
| src/test/java/de/tum/cit/aet/artemis/lti/nightly/NightlyLtiMoodleInteropTest.java | Nightly Moodle 5.0.2 + Postgres Testcontainers interop test driving the full LTI flow. |
| src/test/java/de/tum/cit/aet/artemis/lti/nightly/NightlyLtiCanvasInteropTest.java | Disabled placeholder for future Canvas interop coverage. |
| src/test/java/de/tum/cit/aet/artemis/lti/nightly/NightlyLtiEdxInteropTest.java | Disabled placeholder for future edX interop coverage. |
| src/test/resources/lti/nightly/moodle-sign-jwt.php | CLI helper executed inside the Moodle container to call lti_sign_jwt() for realistic JWT signing. |
| gradle/test.gradle | Excludes nightly-lti tag from the default per-PR run. |
| .github/workflows/nightly-lti-interop.yml | Cron + matrix workflow running the three nightly suites and posting Slack alerts on failure. |
| documentation/docs/developer/lti-nightly-interop-plan.md | Plan document describing the phased coverage approach. |
Comments suppressed due to low confidence (1)
gradle/test.gradle:132
- The
nightly-ltitag is only excluded in therunAllTestsbranch. Running module-targeted tests like./gradlew test -DincludeModules="lti"(line 118-131 branch) or the conveniencetestMysql/testPostgrestasks (line 158-173) will include thenightly-lti-tagged classes and attempt to boot the Moodle/Postgres Testcontainers, breaking these workflows in environments where that is not desired. Consider applyingexcludeTags "nightly-lti"in those branches/tasks as well (or moving the nightly tests out of the default test source set).
if (runAllTests) {
// Default per-PR run: exclude long-running nightly suites. Opt in via -DincludeTags="nightly-lti".
useJUnitPlatform() {
excludeTags "nightly-lti"
}
exclude "**/*IT*", "**/*IntTest*"
} else if (includedModules.size() == 0) {
// not running all tests, but not module-specific ones -> use tags
useJUnitPlatform() {
includeTags includedTags
}
} else {
useJUnitPlatform()
// Always execute "shared"-folder when executing module-specifc tests
includedModules += "shared"
filter { testFilter ->
includedModules.each { val ->
testFilter.includeTestsMatching("de.tum.cit.aet.artemis.$val.*")
}
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
End-to-End Test Results
Test Strategy: Two-phase execution
❌ Failed Tests (Phase 2)
Flakiness Scores for Failed Tests
Overall: ❌ Phase 2 (remaining tests) failed 🔗 Workflow Run · 📊 Test Report Phase 1 · 📊 Test Report Phase 2 |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
WalkthroughThis PR completes Phase 1 and Phase 2 of a phased LTI 1.3 interoperability testing plan. It adds per-PR JWT validation tests exercising a real JWKS HTTP endpoint, a nightly Testcontainers-based Moodle integration suite with in-container JWT signing, updates Lti13LaunchFilter to handle JWT/HTTP exceptions distinctly, configures Gradle to exclude nightly tests by default, and documents the full three-phase approach. The initiation test cleanup removes per-test platform deletion to prevent conflicts with shared integration test state. ChangesLTI Interoperability Testing and Exception Handling
Sequence DiagramsequenceDiagram
participant Test as NightlyLtiMoodleInteropTest
participant Postgres as Postgres Container
participant Moodle as Moodle Container
participant Artemis as Artemis (Lti13LaunchFilter)
Test->>Postgres: start container & wait for health
Test->>Moodle: start container (on shared network)
Test->>Postgres: psql SELECT privatekey, kid
Test->>Moodle: exec moodle-sign-jwt.php (audience, nonce)
Moodle-->>Test: return Moodle-signed id_token
Test->>Artemis: POST /api/lti/public/lti13/auth-login (id_token, state)
Artemis->>Moodle: GET certs.php -> fetch JWKS
Moodle-->>Artemis: JWKS JSON
Artemis->>Artemis: verify RS256 signature & claims
Artemis-->>Test: HTTP 200 (success) or 4xx/5xx (validation failure)
Test->>Postgres: cleanup: DELETE LtiPlatformConfiguration row
Test->>Moodle: stop container
Test->>Postgres: stop container
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PMD (7.24.0)src/test/java/de/tum/cit/aet/artemis/lti/Lti13Step3JwtValidationIntegrationTest.java[WARN] Warning at ruleset.xml:46:5 47| ... [truncated 13303 characters] ... rone.xml/InaccurateNumericLiteral instead of the deprecated Rule name category/ecmascript/errorprone.xml/InnaccurateNumericLiteral. PMD 8.0.0 will remove support for this deprecated Rule name usage. 185| Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@documentation/docs/developer/lti-nightly-interop-plan.md`:
- Around line 303-308: Add a language identifier (for example "text") to the
fenced code block that lists .github/workflows/nightly-lti-interop.yml and the
NightlyLtiMoodleInteropTest.java / MoodleSetup.java entries so the block becomes
```text ... ``` and resolves markdownlint MD040; update the fenced block in the
documentation file where that list appears.
In `@src/main/java/de/tum/cit/aet/artemis/lti/config/Lti13LaunchFilter.java`:
- Line 96: In Lti13LaunchFilter update the log.error call that currently uses
placeholders and passes the exception as an extra parameter; replace the
formatted message with a plain String (e.g., "LTI 1.3 launch request failed with
status: " + ex.getStatusCode().value()) and use the error(String, Throwable)
overload by passing ex as the throwable argument so the call becomes
log.error(<message-without-placeholders>, ex); locate the existing
log.error(...) in Lti13LaunchFilter and make this change.
In
`@src/test/java/de/tum/cit/aet/artemis/lti/Lti13Step3JwtValidationIntegrationTest.java`:
- Line 141: The test currently asserts a generic server error on the POST to
/api/lti/public/lti13/auth-login (the request.performMvcRequest call in
Lti13Step3JwtValidationIntegrationTest), which masks unrelated failures
collapsed into a 500 by Lti13LaunchFilter; replace each
status().is5xxServerError() assertion with a specific expectation that matches
the exact failure under test (e.g., expect a precise HTTP status like
400/401/502 that your filter produces for invalid claims, signature verification
failures, or JWKS fetch errors respectively) and/or assert the response body
contains the specific error identifier/message your filter emits; update all
three occurrences (the performMvcRequest assertions at lines corresponding to
the three tests) to check the exact status and a distinctive response content
fragment rather than a generic 5xx.
In
`@src/test/java/de/tum/cit/aet/artemis/lti/nightly/NightlyLtiMoodleInteropTest.java`:
- Around line 170-177: The test stops the moodle and moodleDb containers but
never closes the Testcontainers Network created via Network.newNetwork(),
leaking Docker networks; update the cleanup() method to close the Network
instance used to start the containers (call network.close() or
network.closeQuietly() on the Network returned by Network.newNetwork()) after
stopping moodle and moodleDb so the custom Docker network is removed; locate the
Network reference created when calling Network.newNetwork() (the variable used
to start moodle/moodleDb) and invoke its close() in cleanup().
- Around line 303-305: The test moodleJwksDocumentIsParseable() currently only
builds a NimbusJwtDecoder via
NimbusJwtDecoder.withJwkSetUri(moodleJwksUri).build() and asserts non-null;
change it to actually fetch/parse the remote JWKS by creating a Moodle-signed
JWT with the existing signWithMoodle(...) helper and calling
decoder.decode(signedJwt) (or decoder.decode(tokenValue)) and asserting the
decode call succeeds (no exception) and returns a Jwt with expected claims; use
the same decoder variable and replace the assertThat(decoder).isNotNull() with
an assert on decoder.decode(...) result.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1217f52d-7b2c-441f-937c-2cec60c85145
⛔ Files ignored due to path filters (1)
.github/workflows/nightly-lti-interop.ymlis excluded by!**/*.yml
📒 Files selected for processing (8)
documentation/docs/developer/lti-nightly-interop-plan.mdgradle/test.gradlesrc/main/java/de/tum/cit/aet/artemis/lti/config/Lti13LaunchFilter.javasrc/test/java/de/tum/cit/aet/artemis/lti/Lti13Step3JwtValidationIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/lti/nightly/NightlyLtiCanvasInteropTest.javasrc/test/java/de/tum/cit/aet/artemis/lti/nightly/NightlyLtiEdxInteropTest.javasrc/test/java/de/tum/cit/aet/artemis/lti/nightly/NightlyLtiMoodleInteropTest.javasrc/test/resources/lti/nightly/moodle-sign-jwt.php
github-project-automation
Bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
Claudia-Anthropica
previously approved these changes
May 24, 2026
Claudia-Anthropica
left a comment
Claudia-Anthropica
left a comment
Contributor
There was a problem hiding this comment.
@krusche Solid PR. The Lti13LaunchFilter fix is correct — HttpStatusException maps to its embedded status (so "Course not found" becomes a real 400) and JwtException rides the wrapped IllegalStateException path to 500, and the catch ordering compiles fine since none of the other caught types is a supertype. The layered per-PR + nightly Moodle coverage is genuinely thorough and well-documented. Just two minor test-quality nits inline, nothing blocking. Heads up that server-tests/client-tests/E2E were still pending when I reviewed, so this assumes they go green.
- Lti13LaunchFilter: also catch IllegalArgumentException so malformed claim shapes from Lti13Service.launchRequestFrom no longer leak past the filter (same servlet-filter escape concern as JwtException and HttpStatusException). - gradle/test.gradle: hoist excludeTags 'nightly-lti' to the testMysql and testPostgres convenience tasks so they don't trigger a 1.8 GB Moodle pull by default. - NightlyLtiMoodleInteropTest: each teardown step in @afterall is now try-wrapped so a docker hiccup in moodle.stop() does not orphan the Postgres container. signWithMoodle now rejects empty / malformed stdout. platformDbId() replaced with the existing findByRegistrationId repository method. Fully qualified class names replaced with imports. fullLaunchSucceeds... now uses UUID-suffixed email + userPrefix so re-runs in the same DB don't short-circuit user creation. - moodle-sign-jwt.php: validate argc, accept optional email arg, and non-zero exit if lti_sign_jwt returns no id_token. Combined with the Java-side check this turns silent-empty-token failures into clear errors. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Align MOODLE_IMAGE to the same tag .env uses (5.0.2-debian-12-r2), removing the divergent source of truth. - Workflow: drop the Canvas/edX matrix entries until their @disabled stubs are implemented (no point burning runners on empty test runs), bump upload-artifact to v7 to match the rest of the repo. - Remove `moodleJwksDocumentIsParseable` — NimbusJwtDecoder.withJwkSetUri is lazy and never hits the network until .decode(), so the original test was a no-op. The Moodle-signed end-to-end test already exercises the real JWKS fetch. - Remove the `ltiPlatformConfigurationRepository` @Autowired in both new test classes — it shadowed the @MockitoSpyBean already provided by AbstractSpringIntegrationIndependentTest, which would have caused the field to bypass the inherited spy reset. - Track Course and User created by fullLaunchSucceedsWith... and delete them in @afterall along with the docker network; the shared LTI base doesn't roll back DB state, so without this they accumulate across nightly invocations sharing the same workspace. - Plan doc: status now reflects shipped state (Phase 1+2 in #12778, Phase 3 deferred). Code fence at line 303 tagged as `text` for markdownlint MD040. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gradle/test.gradle`:
- Around line 163-166: The testMysql and testPostgres tasks currently call
useJUnitPlatform and unconditionally excludeTags "nightly-lti", preventing
-DincludeTags from opting into nightly tests; update their useJUnitPlatform
blocks to mirror the main test task's logic (check runAllTests/project property
"includeTags") so that when runAllTests is true or an includeTags property is
provided you set includedTags = project.findProperty("includeTags") (or
equivalent) and only call excludeTags "nightly-lti" when no includeTags is
present; locate the useJUnitPlatform blocks inside the testMysql and
testPostgres task configurations and apply the same runAllTests/includedTags
branching as the main test task.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9f28a4ea-cd2d-466e-bd9a-bc6f6f1078ae
📒 Files selected for processing (4)
gradle/test.gradlesrc/main/java/de/tum/cit/aet/artemis/lti/config/Lti13LaunchFilter.javasrc/test/java/de/tum/cit/aet/artemis/lti/nightly/NightlyLtiMoodleInteropTest.javasrc/test/resources/lti/nightly/moodle-sign-jwt.php
This @AfterEach was introduced alongside the integration test in #12769 to prevent platform-row leakage, but other LTI tests (LtiIntegrationTest.getAllConfiguredLtiPlatformsAsAdmin and updateLtiPlatformConfigurationAsAdmin) hardcode platform id 1 and implicitly rely on persistent rows — the deleteAll() resets the sequence and causes optimistic-locking failures on subsequent inserts with the same id. The same fix is the entire content of #12772, which now becomes redundant once this PR lands. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Remove the @disabled NightlyLtiCanvasInteropTest and NightlyLtiEdxInteropTest skeleton files. When either LMS gets a real harness, write the test class against the established Moodle pattern instead of resurrecting a stub. - Remove the matrix comment referencing them from nightly-lti-interop.yml. - Plan doc: clarify that no skeleton classes remain. - Lti13Step3JwtValidationIntegrationTest happy path now includes the roles claim so OidcTokenValidator passes and the request reaches lti13Service.performLaunch, which throws BadRequestAlertException ("Course not found") -> 400 via the HttpStatusException catch added earlier. The distinct 400 (vs. 500 for the two rejection tests) satisfies the reviewer concern about all three assertions collapsing onto a generic 5xx. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
|
@Claudia-Anthropica review |
Contributor
|
@krusche acknowledged, queuing this up for immediate review! |
Claudia-Anthropica
approved these changes
May 24, 2026
Claudia-Anthropica
left a comment
Claudia-Anthropica
left a comment
Contributor
There was a problem hiding this comment.
@krusche Re-reviewed after the latest commits and this is in good shape. Both of my earlier notes are addressed — the lazy-decoder isNotNull assertion is gone and the shadowed ltiPlatformConfigurationRepository field is removed. The happy-path Step 3 test now asserts a concrete 400 (course-not-found via BadRequestAlertException), which means it actually drives the JWKS fetch + RS256 verify + claim validation and would catch a future Spring Security regression in NimbusJwtDecoder.withJwkSetUri(...) — that's the real sentinel here, nicely done. The Lti13LaunchFilter catch ordering reads correctly (HttpStatusException -> embedded status before the generic 5xx branch). Approving.
github-project-automation
Bot
moved this from Ready For Review
to Merged
in Artemis Development
2 tasks done
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
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.
Summary
Closes the test gaps that allowed #12739 (LTI/Moodle integration broken by Spring Boot 4) to ship undetected, and hardens
Lti13LaunchFilterso LTI exceptions no longer leak from the servlet pipeline.Adds three layers of LTI 1.3 coverage:
server-tests, ~32s incremental): an in-process JWKS server + signed-id-token test covering the full Step 3b filter chain throughNimbusJwtDecoder.withJwkSetUri(...).bitnamilegacy/moodle:5.0.2+ Postgres via Testcontainers, extracts Moodle's actual RSA signing key from its DB, and exercises every step of the OIDC launch end-to-end against the live LMS — including a full success path with auto-created user, course, and exercise.Lti13LaunchFilternow catchesJwtExceptionandHttpStatusException(e.g.BadRequestAlertException) so signature-verification failures and "Course not found" responses surface with the correct HTTP status instead of leaking as rawRuntimeExceptions.DispatcherServlet'sHandlerExceptionResolverdoes not run for exceptions thrown inside servlet filters, which is why this slipped through.Checklist
General
Server
Motivation and Context
Issue #12739 was caused by the Spring Boot 4 upgrade (#12381) removing
UriComponentsBuilder.fromHttpUrl(String), which the upstreamspring-security-lti13library calls inside the OIDC initiation flow. No server test exercised that code path, so the breakage shipped — Moodle LTI users were locked out until #12769 restored it.#12769 covered Step 1 with an integration test, but two adjacent gaps remained:
NimbusJwtDecoder.withJwkSetUri(...)would slip past CI.While building the nightly test, two further latent bugs surfaced and are fixed:
JwtExceptionfromNimbusJwtDecoderescapedLti13LaunchFilteras an uncaught exception (onlyOAuth2AuthenticationExceptionwas caught).HttpStatusException(carrying its own status code) fromlti13Service.performLaunchalso escaped — every "Course not found" launch was returning an undefined status instead of the 400 the exception specifies.Description
src/main/java/de/tum/cit/aet/artemis/lti/config/Lti13LaunchFilter.java— outercatchnow also catchesJwtException(mapped to 500 via the existing path) andHttpStatusException(mapped to its embedded status, soBadRequestAlertExceptioncorrectly becomes 400).DispatcherServlet's exception resolver does not run for filter-thrown exceptions, which is why these were leaking.Per-PR test (
src/test/java/.../lti/Lti13Step3JwtValidationIntegrationTest.java) — boots a JDKHttpServeron127.0.0.1:0serving a freshly-generated RSA JWKS, signs an id_token with the matching private key, seeds the productionDistributedStateAuthorizationRequestRepository, then POSTs to/api/lti/public/lti13/auth-login. Three tests cover happy path, wrong-key rejection, and expired-token rejection. ~32s total when run as part ofserver-tests(Spring context dominates; per-test cost is <100 ms).Nightly suite (
src/test/java/.../lti/nightly/NightlyLtiMoodleInteropTest.java) — uses Testcontainers to boot Moodle 5.0.2 (bitnamilegacy image, pinned tag) + Postgres on a shared docker network. The PHP signer (src/test/resources/lti/nightly/moodle-sign-jwt.php) isMountableFile-copied into the Moodle container and invoked viadocker exec moodle php /tmp/moodle-sign-jwt.php ...to call Moodle's ownlti_sign_jwt()function. Six tests cover:/mod/lti/auth.phpCourse+OnlineCourseConfiguration+TextExercisefixture — verifies user is auto-created and joined to the course's student groupWorkflow (
.github/workflows/nightly-lti-interop.yml) — cron0 3 * * *UTC + manual dispatch,strategy: matrix:overmoodle/canvas/edx. Posts toLTI_NIGHTLY_SLACK_WEBHOOKon failure. Canvas and edX slots run disabled stubs (@Disabled) — enabling them is one annotation change per file. Operational requirement: addLTI_NIGHTLY_SLACK_WEBHOOKas a repo secret before the first scheduled run.gradle/test.gradle— addsexcludeTags "nightly-lti"to the default per-PR run so the nightly suite stays out of the regular pipeline. Opt in with-DincludeTags='nightly-lti'.Plan document (
documentation/docs/developer/lti-nightly-interop-plan.md) — original design notes; kept as historical context.Steps for Testing
Prerequisites:
server-tests; runs in 32s standalone:bitnamilegacy/moodle:5.0.2image (~1.8 GB, one-time). Subsequent runs reuse the cached image. Wall clock ≈ 60 s (Spring boot ~30 s, Moodle install ~15 s, 6 tests <1 s).Lti13LaunchIntegrationTestcontinues to pass.LTI_NIGHTLY_SLACK_WEBHOOKis configured (via GitHub Actions → Nightly LTI Interop → Run workflow).Testserver States
N/A — this PR only adds tests + a small filter robustness fix. No UI or schema changes.
Review Progress
Code Review
Summary by CodeRabbit
Release Notes
Documentation
Bug Fixes