Repository navigation
Migrate javax.* → jakarta.* (Jakarta EE 9+ / Servlet 6.0 / Tomcat 10.1) - #881
Closed
ismisepaul wants to merge 60 commits into
Closed
ismisepaul wants to merge 60 commits into
ismisepaul wants to merge 60 commits into
Conversation
Build-config-only step of the javax -> jakarta migration. Does not compile on its own (source still imports javax.*); the mechanical import rename follows in the next sub-PR. - jakarta.servlet:jakarta.servlet-api:6.0.0 (was javax.servlet:javax.servlet-api:4.0.1) - jakarta JSTL 3.0 api + glassfish impl (was jstl:jstl:1.2) - org.eclipse.angus:angus-mail:2.0.3 (was com.sun.mail:javax.mail:1.6.2) - remove javax:javaee-api:8.0.1 (pulls the legacy javax namespace back in) - align Spring to 6.1.21 (spring-test 5.3.31, spring-core 6.0.2, spring-context 5.3.31 were inconsistent) - remove spring-mock:2.0.8 (Spring 6 spring-test provides the Jakarta mocks) - remove fongo (unused; tests @disabled, rescue tracked in #876) - .env Tomcat base image 9.0 -> 10.1 (first Jakarta-namespace Tomcat) Refs #861. Targets Spring 6.1.x (not 7) per discussion on #869.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
) * build: jakarta-ready dependencies (PR 1 of jakarta migration #861) Build-config-only step of the javax -> jakarta migration. Does not compile on its own (source still imports javax.*); the mechanical import rename follows in the next sub-PR. - jakarta.servlet:jakarta.servlet-api:6.0.0 (was javax.servlet:javax.servlet-api:4.0.1) - jakarta JSTL 3.0 api + glassfish impl (was jstl:jstl:1.2) - org.eclipse.angus:angus-mail:2.0.3 (was com.sun.mail:javax.mail:1.6.2) - remove javax:javaee-api:8.0.1 (pulls the legacy javax namespace back in) - align Spring to 6.1.21 (spring-test 5.3.31, spring-core 6.0.2, spring-context 5.3.31 were inconsistent) - remove spring-mock:2.0.8 (Spring 6 spring-test provides the Jakarta mocks) - remove fongo (unused; tests @disabled, rescue tracked in #876) - .env Tomcat base image 9.0 -> 10.1 (first Jakarta-namespace Tomcat) Refs #861. Targets Spring 6.1.x (not 7) per discussion on #869. * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…gration #861) (#878) * refactor: javax->jakarta imports in servlets/module/challenge (PR2 #861) Mechanical OpenRewrite rename (JavaxMigrationToJakarta), imports only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor: javax->jakarta imports in servlets/admin (PR2 #861) Mechanical OpenRewrite rename, imports only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor: javax->jakarta imports in remaining servlets (module/lesson, api, root) (PR2 #861) Mechanical OpenRewrite rename, imports only. The invalidate()->logout() behavioral change OpenRewrite bundled into Login/Logout/MobileLogin/SLS/ACS was reverted; these commits are pure import renames. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor: javax->jakarta imports in remaining src/main (PR2 #861) Mechanical OpenRewrite rename, imports only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor: javax->jakarta imports in src/it and src/test (PR2 #861) Mechanical OpenRewrite rename, imports only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
3 tasks
The OpenRewrite rename left jakarta.* imports ordered where javax.* used to sit, which fails google-java-format's import ordering in the lint-java job. Import order only; no code changes. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- jakarta.servlet-api 6.0.0 -> 6.1.0 (Jakarta EE 11) - Tomcat base image 10.1 -> 11.0-jre17-temurin (Servlet 6.1 container) - Spring 6.1.21 -> 7.0.10 via a spring.version property. 6.1 and 6.2 are past OSS support; 7.0 is the supported line and is built against Servlet 6.1. Spring is only used for spring-test's servlet mocks, so spring-context is now test-scoped like the rest. - jstl-api 3.0.0 -> 3.0.2, angus-mail 2.0.3 -> 2.0.5 (patch bumps) Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The retained SAML API blocks clean compilation, and the Docker HTTPS configuration is incompatible with Tomcat 10.1.
3 open issues
What changed in this PR
This integration branch advances Security Shepherd’s Jakarta migration for Spring 6.1, Servlet 6.0, and Tomcat 10.1, combining dependency updates with source and test import changes.
Changes:
- Replace legacy Java EE dependencies with Jakarta Servlet, JSTL, and Mail libraries; align Spring dependencies and remove obsolete mocks.
- Migrate servlet and mail imports while preserving application logic and JDK-owned
javaxpackages. - Switch the Docker Tomcat image to 10.1; JSTL URI modernization and Mongo test restoration remain separate follow-ups.
| File | Description |
|---|---|
| src/test/java/testUtils/TestProperties.java | Migrate test helper servlet import. |
| src/test/java/listeners/DatabaseLifecycleListenerTest.java | Migrate listener test event import. |
| src/main/java/utils/Validate.java | Migrate mail and servlet imports. |
| src/main/java/servlets/SSOMetadata.java | Migrate metadata servlet imports. |
| src/main/java/servlets/SSOLogin.java | Migrate SSO login servlet imports. |
| src/main/java/servlets/SLS.java | Migrate SAML logout servlet imports. |
| src/main/java/servlets/SetupFilter.java | Migrate filter interfaces and HTTP imports. |
| src/main/java/servlets/Setup.java | Migrate setup servlet imports. |
| src/main/java/servlets/Register.java | Migrate registration servlet imports. |
| src/main/java/servlets/module/SolutionSubmit.java | Migrate solution submission servlet imports. |
| src/main/java/servlets/module/RefreshMenu.java | Migrate menu servlet imports. |
| src/main/java/servlets/module/ModuleServletTemplate.java | Migrate module template servlet imports. |
| src/main/java/servlets/module/lesson/XxeLesson.java | Migrate servlet imports; retain JDK XML import. |
| src/main/java/servlets/module/lesson/XssLesson.java | Migrate lesson servlet imports. |
| src/main/java/servlets/module/lesson/UnvalidatedForwardsLesson.java | Migrate lesson servlet imports. |
| src/main/java/servlets/module/lesson/SqlInjectionLesson.java | Migrate lesson servlet imports. |
| src/main/java/servlets/module/lesson/SessionManagementLesson.java | Migrate lesson servlet imports. |
| src/main/java/servlets/module/lesson/SecurityMisconfigLesson.java | Migrate lesson servlet imports. |
| src/main/java/servlets/module/lesson/RedirectLessonTarget.java | Migrate lesson target servlet imports. |
| src/main/java/servlets/module/lesson/Redirect.java | Migrate redirect servlet imports. |
| src/main/java/servlets/module/lesson/PoorValidationLesson.java | Migrate lesson servlet imports. |
| src/main/java/servlets/module/lesson/DirectObjectLesson.java | Migrate lesson servlet imports. |
| src/main/java/servlets/module/lesson/CsrfLessonTarget.java | Migrate lesson target servlet imports. |
| src/main/java/servlets/module/lesson/CsrfLesson.java | Migrate lesson servlet imports. |
| src/main/java/servlets/module/GetModule.java | Migrate module retrieval servlet imports. |
| src/main/java/servlets/module/GetCheat.java | Migrate cheat retrieval servlet imports. |
| src/main/java/servlets/module/FeedbackSubmit.java | Migrate feedback submission servlet imports. |
| src/main/java/servlets/module/challenge/XxeChallenge1OldWebService.java | Migrate and expand servlet imports. |
| src/main/java/servlets/module/challenge/XxeChallenge1.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/XssChallengeTwo.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/XssChallengeThree.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/XssChallengeSix.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/XssChallengeOne.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/XssChallengeFour.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/XssChallengeFive.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/UrlAccess3UserList.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/UrlAccess3.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/UrlAccess2Admin.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/UrlAccess2.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/UrlAccess1Admin.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/UrlAccess1.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SqlInjectionStoredProcedure.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SqlInjectionEscaping.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SqlInjectionEmail.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SqlInjection7.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SqlInjection6.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SqlInjection5VipCheck.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SqlInjection5CouponCheck.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SqlInjection5.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SqlInjection4.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SqlInjection3.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SqlInjection1.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SessionManagement8.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SessionManagement7SecretQuestion.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SessionManagement7.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SessionManagement6SecretQuestion.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SessionManagement6.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SessionManagement5SetToken.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SessionManagement5ChangePassword.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SessionManagement5.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SessionManagement4.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SessionManagement3ChangePassword.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SessionManagement3.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SessionManagement2ChangePassword.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SessionManagement2.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SessionManagement1.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/SecurityMisconfigStealTokens.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/PoorValidation2.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/PoorValidation1.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/NoSqlInjection1.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/DirectObjectBankTransfer.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/DirectObjectBankRegistration.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/DirectObjectBankLogout.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/DirectObjectBankLogin.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/DirectObjectBankCurrentBalance.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/DirectObject2.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/DirectObject1.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/CsrfChallengeTwo.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/CsrfChallengeThree.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/CsrfChallengeTargetTwo.java | Migrate challenge target servlet imports. |
| src/main/java/servlets/module/challenge/CsrfChallengeTargetThree.java | Migrate challenge target servlet imports. |
| src/main/java/servlets/module/challenge/CsrfChallengeTargetSix.java | Migrate challenge target servlet imports. |
| src/main/java/servlets/module/challenge/CsrfChallengeTargetSeven.java | Migrate challenge target servlet imports. |
| src/main/java/servlets/module/challenge/CsrfChallengeTargetOne.java | Migrate challenge target servlet imports. |
| src/main/java/servlets/module/challenge/CsrfChallengeTargetJSON.java | Migrate challenge target servlet imports. |
| src/main/java/servlets/module/challenge/CsrfChallengeTargetFour.java | Migrate challenge target servlet imports. |
| src/main/java/servlets/module/challenge/CsrfChallengeTargetFive.java | Migrate challenge target servlet imports. |
| src/main/java/servlets/module/challenge/CsrfChallengeSixGetToken.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/CsrfChallengeSix.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/CsrfChallengeSevenGetToken.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/CsrfChallengeSeven.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/CsrfChallengeOne.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/CsrfChallengeJSON.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/CsrfChallengeFour.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/CsrfChallengeFive.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/BrokenCryptoHomeMade.java | Migrate servlet imports; retain JDK crypto imports. |
| src/main/java/servlets/module/challenge/BrokenCrypto4.java | Migrate challenge servlet imports. |
| src/main/java/servlets/module/challenge/BrokenCrypto3.java | Migrate challenge servlet imports. |
| src/main/java/servlets/MobileLogin.java | Migrate mobile login servlet imports. |
| src/main/java/servlets/Logout.java | Migrate logout servlet imports. |
| src/main/java/servlets/Login.java | Migrate login servlet imports. |
| src/main/java/servlets/GetJsonScore.java | Migrate score servlet imports. |
| src/main/java/servlets/ChangeUsername.java | Migrate account servlet imports. |
| src/main/java/servlets/ChangePassword.java | Migrate account servlet imports. |
| src/main/java/servlets/api/Scoreboard.java | Migrate servlet imports and annotation. |
| src/main/java/servlets/api/Levels.java | Migrate servlet imports and annotation. |
| src/main/java/servlets/api/Cheats.java | Migrate servlet imports and annotation. |
| src/main/java/servlets/admin/userManagement/UpgradePlayer.java | Migrate user administration servlet imports. |
| src/main/java/servlets/admin/userManagement/UnSuspendUser.java | Migrate user administration servlet imports. |
| src/main/java/servlets/admin/userManagement/SuspendUser.java | Migrate user administration servlet imports. |
| src/main/java/servlets/admin/userManagement/SetDefaultClass.java | Migrate class administration servlet imports. |
| src/main/java/servlets/admin/userManagement/GiveTakePoints.java | Migrate points administration servlet imports. |
| src/main/java/servlets/admin/userManagement/GetPlayersByClass.java | Migrate player listing servlet imports. |
| src/main/java/servlets/admin/userManagement/DowngradeAdmin.java | Migrate user administration servlet imports. |
| src/main/java/servlets/admin/userManagement/DeletePlayers.java | Migrate user administration servlet imports. |
| src/main/java/servlets/admin/userManagement/CreateNewAdmin.java | Migrate user administration servlet imports. |
| src/main/java/servlets/admin/userManagement/CreateClass.java | Migrate class administration servlet imports. |
| src/main/java/servlets/admin/userManagement/ChangeUserPassword.java | Migrate user administration servlet imports. |
| src/main/java/servlets/admin/userManagement/AssignPlayers.java | Migrate player assignment servlet imports. |
| src/main/java/servlets/admin/userManagement/AddPlayer.java | Migrate user administration servlet imports. |
| src/main/java/servlets/admin/moduleManagement/SetModuleStatus.java | Migrate module administration servlet imports. |
| src/main/java/servlets/admin/moduleManagement/RemoveModuleLock.java | Migrate module administration servlet imports. |
| src/main/java/servlets/admin/moduleManagement/OpenWebModules.java | Migrate module administration servlet imports. |
| src/main/java/servlets/admin/moduleManagement/OpenOrCloseByCategory.java | Migrate module administration servlet imports. |
| src/main/java/servlets/admin/moduleManagement/OpenMobileModules.java | Migrate module administration servlet imports. |
| src/main/java/servlets/admin/moduleManagement/OpenAllModules.java | Migrate module administration servlet imports. |
| src/main/java/servlets/admin/moduleManagement/GetProgress.java | Migrate progress servlet imports. |
| src/main/java/servlets/admin/moduleManagement/GetJsonProgress.java | Migrate progress servlet imports. |
| src/main/java/servlets/admin/moduleManagement/GetFeedback.java | Migrate feedback servlet imports. |
| src/main/java/servlets/admin/moduleManagement/EnableModuleBlock.java | Migrate module administration servlet imports. |
| src/main/java/servlets/admin/moduleManagement/CloseAllModules.java | Migrate module administration servlet imports. |
| src/main/java/servlets/admin/config/ToggleRegistration.java | Migrate configuration servlet imports. |
| src/main/java/servlets/admin/config/SetTournamentMode.java | Migrate configuration servlet imports. |
| src/main/java/servlets/admin/config/SetOpenFloorMode.java | Migrate configuration servlet imports. |
| src/main/java/servlets/admin/config/SetCtfMode.java | Migrate configuration servlet imports. |
| src/main/java/servlets/admin/config/SetCountdown.java | Migrate configuration servlet imports. |
| src/main/java/servlets/admin/config/EnableScoreboard.java | Migrate configuration servlet imports. |
| src/main/java/servlets/admin/config/EnableFeedback.java | Migrate configuration servlet imports. |
| src/main/java/servlets/admin/config/EnableCheats.java | Migrate configuration servlet imports. |
| src/main/java/servlets/admin/config/DisableScoreboard.java | Migrate configuration servlet imports. |
| src/main/java/servlets/admin/config/DisableFeedback.java | Migrate configuration servlet imports. |
| src/main/java/servlets/admin/config/DisableCheats.java | Migrate configuration servlet imports. |
| src/main/java/servlets/admin/config/ChangeCoreDatabase.java | Migrate database configuration servlet imports. |
| src/main/java/servlets/ACS.java | Migrate SAML consumer servlet imports. |
| src/main/java/listeners/DatabaseLifecycleListener.java | Migrate listener interfaces and annotation. |
| src/it/java/servlets/SetupIT.java | Migrate integration-test servlet import. |
| src/it/java/servlets/module/lesson/SessionManagementLessonIT.java | Migrate integration-test cookie import. |
| src/it/java/servlets/module/lesson/CsrfLessonIT.java | Migrate integration-test servlet import. |
| src/it/java/servlets/module/GetModuleIT.java | Migrate integration-test servlet import. |
| src/it/java/servlets/module/challenge/NoSqlInjection1IT.java | Migrate integration-test servlet import. |
| src/it/java/servlets/module/challenge/BrokenCryptoHomeMadeIT.java | Migrate integration-test servlet import. |
| src/it/java/servlets/LogoutIT.java | Migrate integration-test session import. |
| src/it/java/servlets/LoginIT.java | Migrate integration-test session import. |
| src/it/java/servlets/api/LevelsIT.java | Migrate integration-test servlet import. |
| src/it/java/servlets/admin/userManagement/DeletePlayersIT.java | Migrate integration-test servlet import. |
| src/it/java/servlets/admin/moduleManagement/OpenOrCloseByCategoryIT.java | Migrate integration-test servlet import. |
| src/it/java/servlets/admin/moduleManagement/OpenAllModulesIT.java | Migrate integration-test servlet import. |
| src/it/java/servlets/admin/moduleManagement/GetJsonProgressIT.java | Migrate integration-test servlet import. |
| src/it/java/servlets/admin/moduleManagement/GetFeedbackIT.java | Migrate integration-test servlet import. |
| src/it/java/servlets/admin/moduleManagement/EnableModuleBlockIT.java | Migrate integration-test servlet import. |
| src/it/java/servlets/admin/moduleManagement/CloseAllModulesTestIT.java | Migrate integration-test servlet import. |
| src/it/java/servlets/admin/config/ToggleRegistrationIT.java | Migrate integration-test servlet import. |
| src/it/java/servlets/admin/config/SetTournamentModeIT.java | Migrate integration-test servlet import. |
| src/it/java/servlets/admin/config/SetOpenFloorModeIT.java | Migrate integration-test servlet import. |
| src/it/java/servlets/admin/config/SetCtfModeIT.java | Migrate integration-test servlet import. |
| src/it/java/servlets/admin/config/EnableFeedbackIT.java | Migrate integration-test servlet import. |
| src/it/java/servlets/admin/config/EnableCheatsIT.java | Migrate integration-test servlet import. |
| src/it/java/servlets/admin/config/DisableScoreboardIT.java | Migrate integration-test servlet import. |
| src/it/java/servlets/admin/config/DisableFeedbackIT.java | Migrate integration-test servlet import. |
| src/it/java/servlets/admin/config/DisableCheatsIT.java | Migrate integration-test servlet import. |
| pom.xml | Update Jakarta and Spring dependencies; remove obsolete mocks. |
| .env | Select Tomcat 10.1 with Java 17. |
Review effort: Balanced
) java-saml has no usable Jakarta release (its Auth class only accepts javax.servlet types), which left ACS, Logout, SLS and SSOLogin unable to compile. SSO now runs on pac4j-saml 6.6 (OpenSAML 5, Jakarta-native, itself built on Spring 7). - utils.SamlSso builds the SAML2Client from the existing onelogin.saml.properties keys, so deployments keep their config: the SP's PEM cert/key go into an in-memory keystore (PKCS#8 or PKCS#1), and the IdP settings are turned into an IdP metadata document. Nothing is written to disk. - utils.SsoUserMapper holds the attribute -> user/role mapping that used to be inline in ACS, now unit-testable. - The five servlets delegate to SamlSso; the Getter.authUserSSO hand-off and session setup are unchanged. Bugs fixed along the way (each covered by a test): - SLS only defined handleRequest(), so /saml/sls answered 405 to every IdP logout message. It now handles GET and POST. - Logout read the NameID/session index from the freshly created session after invalidating the old one, so SLO requests carried no NameID. ACS now stores the SAML profile in the post-login session and Logout reads it before invalidating. - ACS hit a NullPointerException when a configured attribute was missing, and left a blank page when the assertion had no attributes; both now return to the login page with a message. Build: spring-core is no longer declared test-scoped (that would strip pac4j's compile-scope copy from the WAR); a spring-framework-bom import keeps every Spring module on ${spring.version}. commons-io 2.18 -> 2.22 to match what pac4j is built against. log4j-slf4j2-impl routes pac4j / OpenSAML logging into log4j2. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…R 6 of jakarta migration #861) - web.xml: Java EE 2.5 descriptor -> Jakarta EE web-app 6.1 (https://jakarta.ee/xml/ns/jakartaee). Annotation scanning was already on under 2.5 (the /api servlets rely on @WebServlet), so metadata-complete is left unset. - docker/tomcat/serverxml.patch regenerated against Tomcat 11.0.27's conf/server.xml: the old 8080-connector hunk no longer matched, which would fail `RUN patch` in the image build. The HTTPS connector now uses <SSLHostConfig>/<Certificate> instead of the legacy Connector-level keystoreFile/keystorePass/keyAlias attributes; the resulting setup is the same (8080 redirects to HTTPS_PORT, TLS on 8443 from the built keystore). - Dockerfile placeholder substitution updated for the new attribute names. webxml.patch still applies cleanly to Tomcat 11.0.27's conf/web.xml. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#861) translation.jsp is the only JSP declaring taglibs; the seven JSTL pages (index, login, register, setup, getStarted, readyToPlay, blockedMessage) include it. JSTL 3.0 still resolves the legacy java.sun.com URIs, so this is modernisation rather than a fix. The jakarta.tags.core/fmt URIs are declared in the bundled jakarta.servlet.jsp.jstl 3.0.1 TLDs. Closes #880 Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… 8 of jakarta migration #861) - AGENTS.md said the project targets Java 8; it has targeted Java 17 since the JDK 17 upgrade (#847). Documents the Jakarta EE 11 / Tomcat 11 platform, which javax packages remain fine, the jakarta.tags.* taglibs, Spring being test-only, and the pac4j-based SAML SSO and its configuration files. - docs/testing.md: JDK 8 -> JDK 17 prerequisite. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The old patch was a diff against a 2020 Tomcat web.xml. On Tomcat 11's conf/web.xml, GNU patch (used in the image build) only applied it with fuzz 2 at an offset of -4108 lines, dropping the security-constraint between session-config and the MIME mappings. That is still valid XML and still enforced HTTPS, but only by luck. Regenerated as an exact diff against the base image's conf/web.xml; verified with the image's own GNU patch (Hunk #1 succeeded, no fuzz) alongside serverxml.patch. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- README / AGENTS: an existing Docker environment keeps its old Tomcat
9/10.1 conf/ in the named conf volume, so Tomcat 11 would start with
stale server.xml / web.xml. Remove the volume when upgrading; data
volumes are untouched.
- AGENTS: the it-test profile tags its Mongo build as
mongo:${MONGODB_VERSION}, overwriting the official image locally so
the next IT run fails on chown. Documents the re-pull workaround.
Refs #861
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ismisepaul
added this pull request to stack #943
October 22, 2026 21:01
…optional SP key Addresses review on #939. - Sessions are registered and ended by the complete NameID: value, Format, NameQualifier and SPNameQualifier, with omitted fields taking their SAML defaults (unspecified format; IdP / SP entity IDs as qualifiers, SAML Core 8.3.7). A NameID-only logout can no longer end sessions of a different principal that happens to share the value. - pac4j's SP metadata advertised HTTP-POST-SimpleSign and SOAP logout endpoints, which SamlSso can't verify signatures for; an IdP choosing them would have its signed logouts rejected. The metadata now lists only HTTP-POST and HTTP-Redirect (filtered before metadata signing). - Like java-saml, the SP certificate/key may be omitted when the SP signs nothing (authnrequest_signed, logoutrequest_signed and sign_metadata all false); a temporary in-memory key pair satisfies pac4j. Setting only one of the two, or omitting them while a signing option is on, fails with a message naming the settings. Regression tests for each; each fails with its fix switched off. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Addresses review on #939. pac4j 6.6 rejects a response unless every requested AuthnContextClassRef appears in it, but SAML treats the requested classes as alternatives (SAML Core 3.3.2.2.1). With two classes configured, a valid signed response carrying one of them was rejected and ACS reported "Not authenticated". SamlSso now uses a SAML2Client whose response validator requires, for "exact" comparison (the default), at least one returned class to be one of those requested. For minimum / better / maximum the IdP judges relative strength, which an SP can't check in general (a stronger class may be one that wasn't listed), so the returned class is accepted, as java-saml did. Tests: one-of-several accepted, unrequested class rejected under exact, stronger unlisted class accepted under minimum, any class accepted when none is requested. The two acceptance cases fail with pac4j's validator. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Addresses review on #939. With onelogin.saml2.sp.single_logout_service.url empty, pac4j advertised the ACS URL as the single logout endpoint, but ACS doesn't process logout messages, so an IdP following the metadata would send LogoutRequests that left sessions active. The SP metadata now lists no SingleLogoutService in that case, and SamlSso refuses IdP LogoutRequests rather than handling them against the ACS URL. The sample config documents leaving the setting empty to disable IdP-initiated logout. Tests: metadata without the SLS setting has no logout endpoint, and an IdP LogoutRequest is refused without ending sessions. Both fail under the previous behaviour. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Addresses review on #942: the backup was written to the repository root, which is the Tomcat image's build context. On Linux the archive is root-owned and mode 600, which can break the next build, and the secrets were sent into the build context. The backup now goes to a mode-700 ~/shepherd-conf-backup directory outside the repository, which the user can delete even though the archive is root-owned. Backup and restore commands checked end to end on a scratch volume. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ismisepaul
removed this pull request from stack #943
October 23, 2026 06:26
ismisepaul
added this pull request to stack #945
October 23, 2026 06:27
ismisepaul
removed this pull request from stack #945
October 23, 2026 06:28
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The broad platform upgrade and security-sensitive SAML replacement require final human review; the logout regression-test request remains a non-blocking nit.
No open issues 🎉
3 resolved since last review
Review effort: Balanced
pac4j-saml depends on OpenSAML 5, which Shibboleth publishes only to its own repository (build.shibboleth.net), not Maven Central. That repository was previously trusted implicitly via pac4j-saml's POM, for any artifact, with checksum mismatches only warned about. A CI build on #881 failed when it timed out. - Declare shibboleth-releases explicitly in pom.xml with checksumPolicy=fail. - Enable Maven remote repository filtering (.mvn/maven.config) so that repository may serve only org.opensaml and net.shibboleth artifacts (.mvn/rrf/groupId-shibboleth-releases.txt); everything else resolves from Central only, closing off dependency confusion through it. Verified from an empty local repository: resolves with the allow-list, and refuses to fetch OpenSAML when org.opensaml is removed from it. - AGENTS.md documents the repository, the filter, and what a resolution failure from it means. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
Author
ismisepaul
added a commit
that referenced
this pull request
Oct 23, 2026
Addresses review on #881. Maven before 3.9.2 does not expand ${maven.multiModuleProjectDirectory} in .mvn/maven.config, so the remote-repository filter's allow-list is never found and the filter fails open. Checked against real releases: 3.9.0 and 3.9.1 fetched OpenSAML from Shibboleth with org.opensaml removed from the allow-list; 3.9.2, 3.9.4, 3.9.8, 3.9.16 and 3.10.0 refused it. A relative path is resolved against the local repository and fails open on every version. - maven-enforcer-plugin (first plugin in the build) requires [3.9.2,), so older Maven stops with an explicit message. - pgp-verification-test.sh gains step 0: with org.opensaml removed from the allow-list, resolution must fail and nothing may be downloaded from Shibboleth for org.opensaml. Run in CI, this proves the filter is enforced on the runner's own Maven. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ismisepaul
added a commit
that referenced
this pull request
Oct 23, 2026
) (#939) * feat: reimplement SAML SSO on pac4j-saml (PR 5 of jakarta migration #861) java-saml has no usable Jakarta release (its Auth class only accepts javax.servlet types), which left ACS, Logout, SLS and SSOLogin unable to compile. SSO now runs on pac4j-saml 6.6 (OpenSAML 5, Jakarta-native, itself built on Spring 7). - utils.SamlSso builds the SAML2Client from the existing onelogin.saml.properties keys, so deployments keep their config: the SP's PEM cert/key go into an in-memory keystore (PKCS#8 or PKCS#1), and the IdP settings are turned into an IdP metadata document. Nothing is written to disk. - utils.SsoUserMapper holds the attribute -> user/role mapping that used to be inline in ACS, now unit-testable. - The five servlets delegate to SamlSso; the Getter.authUserSSO hand-off and session setup are unchanged. Bugs fixed along the way (each covered by a test): - SLS only defined handleRequest(), so /saml/sls answered 405 to every IdP logout message. It now handles GET and POST. - Logout read the NameID/session index from the freshly created session after invalidating the old one, so SLO requests carried no NameID. ACS now stores the SAML profile in the post-login session and Logout reads it before invalidating. - ACS hit a NullPointerException when a configured attribute was missing, and left a blank page when the assertion had no attributes; both now return to the login page with a message. Build: spring-core is no longer declared test-scoped (that would strip pac4j's compile-scope copy from the WAR); a spring-framework-bom import keeps every Spring module on ${spring.version}. commons-io 2.18 -> 2.22 to match what pac4j is built against. log4j-slf4j2-impl routes pac4j / OpenSAML logging into log4j2. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: authenticate SAML logout messages and end only the named sessions Addresses review on #939. Logout message trust: - pac4j validates a logout message's XML signature only when one is present, and never checks HTTP-Redirect query-string signatures (the configured SLO binding). Unsigned or forged LogoutRequests and LogoutResponses were therefore accepted. When want_messages_signed is set, SamlSso now requires a valid IdP signature: XML (HTTP-POST) or query-string (HTTP-Redirect, verified per SAML Bindings 3.4.4.1 with the configured IdP certificate; SHA-1 algorithms are not accepted). Session targeting: - SLS invalidated whichever session the delivering browser had. An IdP LogoutRequest now ends exactly the sessions created by the login it names (NameID + SessionIndex), via a new SamlSessionRegistry that ACS registers the post-login session in. This works whichever browser delivers the message and with no session cookie. An unsigned request carrying only a NameID ends nothing. - A LogoutResponse ends nothing: Logout already ended the local session before sending the user to the IdP. Also: - Send NameQualifier / SPNameQualifier back in LogoutRequests (useNameQualifier), so IdPs issuing qualified NameIDs can match them. - Default requested_authncontextcomparison to "exact" (pac4j drops RequestedAuthnContext entirely without one), and still read the key as misspelled in the old sample file. - @OverRide on SLS doGet/doPost. Tests: SamlLogoutTest covers signed/unsigned/unknown-key/tampered LogoutResponses and LogoutRequests in both bindings, session targeting across browsers and without a cookie, and SessionIndex mismatch; plus NameID qualifier and comparison-default tests. Each fix was checked to fail its tests when switched off. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: bind SAML redirect signatures to the message pac4j reads; end every SessionIndex Addresses review on #939. - The HTTP-Redirect signature check matched query parameters by their raw names, while the servlet container (and pac4j) URL-decodes names. A captured signed query with an unsigned message prepended as %53AMLRequest=... passed the check on the signed copy while pac4j processed the unsigned one. The check now accepts only a GET, rejects percent-encoded aliases and repeated signed parameters, and requires each signed value to equal the single value the container returns. - An IdP LogoutRequest naming several SessionIndexes ended only the first; it now ends each one. A NameID-wide logout (no SessionIndex) still requires a signed request. Regression tests (the slsGet fixture now URL-decodes parameter names like a container): encoded alias, repeated parameter, redirect signature on a POST, and two SessionIndexes for one NameID. Three of the four fail against the previous SamlSso; the repeated-parameter case already failed safe and is kept as a guard. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * build: correct the spring.version comment Spring is no longer test-only: pac4j-saml needs it at runtime and SamlSso uses spring-core's ByteArrayResource (review on #942). Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: check SAML LogoutRequest destination, freshness and replay; ECDSA format Addresses review on #939. pac4j 6.6's LogoutRequest validation checks signature (if present) and issuer only, so before a LogoutRequest may end sessions SamlSso now checks: - Destination: a signed request must be addressed to this SP's logout endpoint (SAML Bindings 3.4.5.2 / 3.5.5.2), and any Destination given must match. A request signed for another SP that shares NameIDs can no longer end Shepherd sessions. - Freshness: IssueInstant within 5 minutes (plus 5 minutes clock skew), and NotOnOrAfter not passed. - Replay: request IDs (per issuer) are recorded atomically in a bounded cache that outlives the acceptance window, so a captured request can't be replayed to end a later login. Also: XML-DSig ecdsa-sha* redirect signatures are raw r || s (IEEE P1363), not DER; verify them with the SHA*withECDSAinP1363Format JCA algorithms so EC-signing IdPs work. Regression tests: wrong / missing Destination, stale, future-dated and expired requests, replay after a fresh login, and an EC IdP's ecdsa-sha256 redirect signature. All seven fail against the previous SamlSso. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: match complete SAML NameIDs, advertise verifiable SLO bindings, optional SP key Addresses review on #939. - Sessions are registered and ended by the complete NameID: value, Format, NameQualifier and SPNameQualifier, with omitted fields taking their SAML defaults (unspecified format; IdP / SP entity IDs as qualifiers, SAML Core 8.3.7). A NameID-only logout can no longer end sessions of a different principal that happens to share the value. - pac4j's SP metadata advertised HTTP-POST-SimpleSign and SOAP logout endpoints, which SamlSso can't verify signatures for; an IdP choosing them would have its signed logouts rejected. The metadata now lists only HTTP-POST and HTTP-Redirect (filtered before metadata signing). - Like java-saml, the SP certificate/key may be omitted when the SP signs nothing (authnrequest_signed, logoutrequest_signed and sign_metadata all false); a temporary in-memory key pair satisfies pac4j. Setting only one of the two, or omitting them while a signing option is on, fails with a message naming the settings. Regression tests for each; each fails with its fix switched off. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: treat requested SAML authentication contexts as alternatives Addresses review on #939. pac4j 6.6 rejects a response unless every requested AuthnContextClassRef appears in it, but SAML treats the requested classes as alternatives (SAML Core 3.3.2.2.1). With two classes configured, a valid signed response carrying one of them was rejected and ACS reported "Not authenticated". SamlSso now uses a SAML2Client whose response validator requires, for "exact" comparison (the default), at least one returned class to be one of those requested. For minimum / better / maximum the IdP judges relative strength, which an SP can't check in general (a stronger class may be one that wasn't listed), so the returned class is accepted, as java-saml did. Tests: one-of-several accepted, unrequested class rejected under exact, stronger unlisted class accepted under minimum, any class accepted when none is requested. The two acceptance cases fail with pac4j's validator. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: advertise no SAML logout endpoint when no SLS URL is configured Addresses review on #939. With onelogin.saml2.sp.single_logout_service.url empty, pac4j advertised the ACS URL as the single logout endpoint, but ACS doesn't process logout messages, so an IdP following the metadata would send LogoutRequests that left sessions active. The SP metadata now lists no SingleLogoutService in that case, and SamlSso refuses IdP LogoutRequests rather than handling them against the ACS URL. The sample config documents leaving the setting empty to disable IdP-initiated logout. Tests: metadata without the SLS setting has no logout endpoint, and an IdP LogoutRequest is refused without ending sessions. Both fail under the previous behaviour. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * build: restrict and PGP-verify the Shibboleth repository (OpenSAML) pac4j-saml depends on OpenSAML 5, which Shibboleth publishes only to its own repository (build.shibboleth.net), not Maven Central. That repository was trusted implicitly via pac4j-saml's POM, for any artifact, with checksum mismatches only warned about, and nothing checked signatures. - pom.xml declares shibboleth-releases with checksumPolicy=fail. - Maven remote repository filtering (.mvn/maven.config, .mvn/rrf/groupId-shibboleth-releases.txt) lets that repository serve only org.opensaml and net.shibboleth; everything else comes from Central only. - pgpverify-maven-plugin checks every dependency's PGP signature on every build (disableChecksum=true, so a jar tampered with in the local repository after a successful build is still caught). .mvn/pgp-keys.map pins org.opensaml and net.shibboleth to the Shibboleth release key (B5B5DD332142AD657E8D87AC7D27E610B8A3DC52, as published at shibboleth.net/downloads/PGP_KEYS); other groupIds are listed by name and need a valid signature. There is deliberately no wildcard rule: any matching rule lets an artifact pass, so a catch-all would undo the pin (caught by the test below). The one unsigned legacy artifact, hamcrest-core 1.1 via json-simple (#944), is allowed by exact version. - src/test/scripts/pgp-verification-test.sh, run in the CI build job: genuine artifacts pass; OpenSAML pinned to a different key fails ("Not allowed artifact org.opensaml..."); a tampered opensaml-core-api jar fails ("PGP Signature INVALID"). - CI's change filter now includes .mvn/**. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * build: require Maven 3.9.2+ so the Shibboleth repository filter applies Addresses review on #881. Maven before 3.9.2 does not expand ${maven.multiModuleProjectDirectory} in .mvn/maven.config, so the remote-repository filter's allow-list is never found and the filter fails open. Checked against real releases: 3.9.0 and 3.9.1 fetched OpenSAML from Shibboleth with org.opensaml removed from the allow-list; 3.9.2, 3.9.4, 3.9.8, 3.9.16 and 3.10.0 refused it. A relative path is resolved against the local repository and fails open on every version. - maven-enforcer-plugin (first plugin in the build) requires [3.9.2,), so older Maven stops with an explicit message. - pgp-verification-test.sh gains step 0: with org.opensaml removed from the allow-list, resolution must fail and nothing may be downloaded from Shibboleth for org.opensaml. Run in CI, this proves the filter is enforced on the runner's own Maven. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: keep no-op logouts out of the replay cache; omit temporary SP keys Addresses review on #939. - With want_messages_signed=false, an unsigned LogoutRequest that ends nothing (NameID only, or a SessionIndex we don't track) was still recorded in the bounded replay cache, so a flood of them could fill it and make genuine signed logouts be refused. Replay recording now happens only for requests that will act: signed requests, or unsigned ones naming a tracked login (SamlSessionRegistry.isRegistered). The destination/freshness checks still apply to every request. - With no SP certificate configured, the temporary in-memory key was still published in SP metadata; an IdP encrypting to it would break after a restart. Metadata now has no KeyDescriptor in that case; configured certificates are still published. Tests: a flood of unsigned no-op logouts against a small cache no longer blocks a signed logout; an unsigned logout naming a tracked login still ends it; keyless metadata publishes no key, configured metadata does. The flood and keyless cases fail with their fixes switched off. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: SLS must not log users out by redirecting to login.jsp Addresses review on #939. login.jsp invalidates the current session when no login/registration message is pending, so SLS redirecting there after an empty or rejected logout message let any page log a user out by sending their browser to /saml/sls. A LogoutResponse delivered through a different, still-logged-in browser did the same. - Invalid or missing logout message: 400 Bad Request, no redirect. - LogoutResponse: redirect to index.jsp if the browser's session is still logged in, else login.jsp. Tests (SLSTest): GET/POST without a message return 400 with no redirect; an invalid message keeps the session and does not redirect; a LogoutResponse keeps a logged-in session (index.jsp) and sends a logged-out browser to login.jsp. Four of the five fail against the previous SLS. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: bind SAML logins to the browser, gate SSO on sso.enabled, reject blank IDs Addresses review on #939. Login CSRF (P1): pac4j's default message store keeps nothing and it accepts responses without InResponseTo, so nothing tied a login response to the browser that started the login. A valid response for an attacker's account could be posted through a victim's browser and sign it into that account. - SamlRequestStore records each sent AuthnRequest bound to a random token in a dedicated cookie (HttpOnly, Secure, SameSite=None so it survives the IdP's cross-site POST, scoped to the ACS path; the session cookie is untouched). pac4j only accepts a Response whose InResponseTo names a pending request bound to the delivering browser; each request is used once and expires after 10 minutes. - Unsolicited (IdP-initiated) responses, which carry no InResponseTo and can't be bound to a browser, are rejected. Also: - ACS, SSOLogin, SLS and SSOMetadata answer 404 unless SSO is enabled (LoginMethod.isSaml, as Login already checks), so a retained SAML configuration can't authenticate anyone while sso.enabled is off. - SsoUserMapper treats blank (empty / whitespace-only) identifiers and display names as missing, so distinct principals can't share an account through a blank ssoName; non-blank values are kept as sent. Tests: the fixture now runs a real SP-initiated login (startLogin) and answers it, so the existing authentication tests exercise the full round trip; no assertion was weakened. New: unsolicited response rejected; response delivered through another browser (no cookie, or its own) rejected; response answering another browser's login rejected; all four SSO endpoints 404 with SSO disabled, and ACS doesn't authenticate; blank ssoName/userName rejected, non-blank kept as sent. Each fix fails its tests when switched off. 251 unit and 312 integration tests pass. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: SAML sample notes SP-initiated-only login and the binding cookie IdP-initiated (unsolicited) responses are now refused and logins are bound to the browser by the shepherd_saml_request cookie; say so where operators configure SSO. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: scope the SAML binding cookie to the SAML directory; isolate fixture state Addresses review on #939. - The binding cookie had Path=<ACS path> (/saml/acs), so /saml/dologin never received it: every new login issued a fresh token that replaced the previous one, and with two tabs logging in at once the first callback was rejected. The cookie is now scoped to the ACS URL's directory (e.g. /saml/, keeping any context prefix), so a second login reuses the browser's token. - Test fixture: the simulated browser state (pending AuthnRequest ID, binding cookie) is now cleared whenever a client is installed, so it can't leak between tests; and the browser sends its cookie only where the cookie's Path matches, instead of tests copying it by hand. authenticate_ResponseToAnotherBrowsersLogin_IsRejected now switches to a separate browser explicitly (newBrowser()) before the victim's login; its assertion is unchanged. Tests: two overlapping logins in one browser both succeed; the cookie is Path=/saml/, HttpOnly, Secure, SameSite=None. Both fail with the old ACS-only path. SamlSsoTest passes in three random method orders. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: refresh the SAML binding cookie's Max-Age on every login start A reused binding token was not re-sent, so the cookie kept the expiry of the first login. A second login started near that expiry had its callback arrive without the cookie and was rejected. The cookie is now re-sent on every login with a full lifetime. The test fixture models the browser dropping cookies by Max-Age to cover this. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: require the bearer confirmation to answer the correlated request pac4j binds a SAML Response to the browser's pending AuthnRequest only through the Response's InResponseTo, which is unsigned unless the whole message is signed (want_messages_signed defaults to false), and never checks the InResponseTo of the bearer SubjectConfirmationData. A valid assertion for one account could therefore be placed in an envelope naming another browser's pending request and log that browser in. The bearer confirmation, inside the signed assertion, must now name the same request as the Response (SAML Profiles 4.1.4.3). A confirmation with no InResponseTo is refused too, since only SP-initiated login is supported. Decrypted assertions go through the same check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: note the bearer confirmation must name the AuthnRequest Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: give each SAML login its own binding cookie All logins shared one binding cookie. Two tabs that opened the login page before either had the cookie each got a different token, and the second Set-Cookie replaced the first, so the first tab's login was rejected. Each AuthnRequest now gets its own cookie, named after a hash of its ID, which lives as long as the request and is deleted once the login completes. That also replaces the Max-Age refresh needed by the shared cookie. The test fixture now keeps a cookie jar and can start logins in parallel tabs. The cross-browser rejection test uses the per-request cookie name, so it still presents a cookie of the right name with the wrong token. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.
Jakarta migration integration branch (#861)
Integration branch consolidating the stacked sub-PRs of the
javax.* → jakarta.*migration. Opening as draft for review/CI before targetingdev.What's included
pom.xml+.env):jakarta.servlet:jakarta.servlet-api:6.0.0(Servlet 6.0)org.eclipse.angus:angus-mail:2.0.3(Jakarta Mail)javax:javaee-api(pulled the legacy namespace onto the classpath)spring-mockand unusedfongo(rescue tracked in Modernize MongoDatabase off legacy com.mongodb.DB API + Testcontainers; rescue disabled MongoDatabaseIT tests #876)9.0→10.1-jre17-temurinjavax.* → jakarta.*import rename across 170 source/test/IT files.Verification
javax.servlet, 170 onjakarta.servlet.mvn compileandmvn test-compileboth pass locally on Temurin 17.devis the 170 renames +pom.xml+.env.Notes
jakarta.tags.*in PR 7 (refactor: JSTL taglib URIs to jakarta.tags.* (PR 7 of jakarta migration #861) #941, closes Modernize JSTL taglib URIs to jakarta.tags.* (post-jakarta migration) #880).Remaining stack (merge in order, with merge commits)
lint-javaweb.xml6.1 schema and Tomcat 11 config patchesjakarta.tags.*Verified at the top of the stack: unit tests (192), integration tests (312) and a Docker smoke test on Tomcat 11.0.27.
Closes #861.
🤖 Generated with Claude Code