Repository navigation
Comparing changes
Open a pull request
base repository: OWASP/SecurityShepherd
base: dev@{1day}
head repository: OWASP/SecurityShepherd
compare: dev
- 7 commits
- 202 files changed
- 4 contributors
Commits on Oct 23, 2026
-
build: adopt Semantic Versioning, start 4.0.0-SNAPSHOT (#951)
- Bump pom.xml from 3.1.0-SNAPSHOT to 4.0.0-SNAPSHOT. The Jakarta migration (#861) is a breaking change, so the next release is 4.0.0. - release.yml also runs on plain X.Y.Z tag pushes and fails if the tag doesn't match the pom.xml version. Release tags publish Docker images X.Y.Z, X.Y, X and latest; branch pushes still publish dev/master. - Document the versioning scheme and release steps in docs/releasing.md, CONTRIBUTING.md and AGENTS.md. Refs #949 Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Configuration menu - View commit details
-
Copy full SHA for 34847fe - Browse repository at this point
Copy the full SHA 34847feView commit details -
build: Jakarta EE namespace migration and lint fix (PRs 1-3 of jakart…
…a migration #861) (#946) * 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> * build: jakarta-ready dependencies (PR 1 of jakarta migration #861) (#877) * 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> * refactor: mechanical javax->jakarta import rename (PR 2 of jakarta migration #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> * style: re-sort imports after javax->jakarta rename (spotless) 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> * build: make the bottom of the stack build on its own (Tomcat 10.1, no java-saml) Every PR in the Jakarta migration stack must pass CI by itself. This one didn't compile: ACS, Logout, SLS and SSOLogin passed jakarta servlet types to java-saml 2.9.0, which only accepts javax.servlet and has no Jakarta release. Its Docker build would also have failed: the Tomcat 9 era server.xml patch doesn't apply to Tomcat 10.1. - Remove java-saml. The SSO endpoints (ACS, SSOLogin, SLS, SSOMetadata) answer 503 "SAML SSO is unavailable during migration", and Logout drops its java-saml branch. SSO is off unless sso.enabled=true, and is reimplemented on pac4j-saml in #939 further up the stack, so the final code on dev is unchanged. - Regenerate serverxml.patch / webxml.patch against Tomcat 10.1.61's conf files (HTTPS via <SSLHostConfig>/<Certificate>), with the matching Dockerfile substitutions. Both apply exactly with the image's GNU patch. Verified locally: spotless, 169 unit tests, 312 integration tests, mvn -Pdocker install, docker compose build, and a Tomcat 10.1.61 smoke test (deploys, HTTP->HTTPS, TLS 1.3, login page, SSO endpoints 503). Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: substitute HTTPS_PORT when redirectPort is on its own line Addresses review on #946. Tomcat 10.1's server.xml puts the HTTP connector's attributes on separate lines, so in the regenerated serverxml.patch redirectPort="HTTPS_PORT" is not followed by " />". The Dockerfile's sed required that suffix, left the literal HTTPS_PORT in conf/server.xml, and HTTP->HTTPS redirects ignored the configured port (it went unnoticed with the default 443). The sed now matches just the attribute, which works for both the Tomcat 10.1 and 11 patch layouts. Test: src/test/scripts/tomcat-config-test.sh, run in CI's build job after docker compose build, checks the built image's conf/server.xml redirects to HTTPS_PORT and has no HTTPS_PORT / TLS_KEYSTORE_* / ALIAS placeholders left. It fails on the previous Dockerfile (HTTPS_PORT=8443) and passes with the fix for 8443 and 443. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: SeanDuggan <sean.duggan4@gmail.com>
Configuration menu - View commit details
-
Copy full SHA for 0558c59 - Browse repository at this point
Copy the full SHA 0558c59View commit details -
build: target Jakarta EE 11 / Servlet 6.1 / Tomcat 11 / Spring 7 (PR …
…4 of jakarta migration #861) (#936) * build: target Jakarta EE 11 (Servlet 6.1, Tomcat 11, Spring 7) - 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> * build: Tomcat 11 config patches alongside the Tomcat 11 image This PR moves the base image to Tomcat 11, but the server.xml / web.xml patches applied during the image build were still written against older Tomcat config files. On Tomcat 11, serverxml.patch fails one hunk, so `docker compose build` (the CI build job) failed on every branch between this one and PR 6, which regenerated them. Moves those changes here, next to the image change they depend on: - serverxml.patch regenerated against Tomcat 11.0.27; HTTPS connector uses <SSLHostConfig>/<Certificate> instead of the legacy Connector keystore attributes - webxml.patch regenerated as an exact diff (GNU patch previously needed fuzz 2 at a -4108 line offset) - Dockerfile placeholder substitution for the new attribute names Content is identical to PR 6's; merging up the stack is a no-op there. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Configuration menu - View commit details
-
Copy full SHA for ef571f7 - Browse repository at this point
Copy the full SHA ef571f7View commit details -
feat: reimplement SAML SSO on pac4j-saml (PR 5 of jakarta migration #861
) (#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>
Configuration menu - View commit details
-
Copy full SHA for c462ad7 - Browse repository at this point
Copy the full SHA c462ad7View commit details -
build: web.xml on the Jakarta EE 11 (Servlet 6.1) schema (PR 6 of jak…
…arta migration #861) (#940) * build: web.xml on Jakarta EE 11 schema, Tomcat 11 server.xml patch (PR 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> * build: regenerate webxml.patch against Tomcat 11.0.27 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> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Configuration menu - View commit details
-
Copy full SHA for cb548ee - Browse repository at this point
Copy the full SHA cb548eeView commit details -
refactor: JSTL taglib URIs to jakarta.tags.* (PR 7 of jakarta migration
#861) (#941) 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>
Configuration menu - View commit details
-
Copy full SHA for 7f26fec - Browse repository at this point
Copy the full SHA 7f26fecView commit details -
docs: Java 17 / Jakarta EE 11 / Tomcat 11, pac4j SAML, upgrade notes …
…(PR 8 of jakarta migration #861) (#942) * docs: document Java 17 / Jakarta EE 11 / Tomcat 11 and pac4j SAML (PR 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> * docs: upgrade note for the Tomcat conf volume, Mongo IT image workaround - 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> * docs: back up the Tomcat conf volume before recreating it Addresses review on #942: the conf volume also holds the TLS keystore and database.properties / mongo.properties, not just Tomcat's XML. The upgrade steps now back the volume up first and explain restoring only a customised keystore or connection settings, never the old server.xml / web.xml. Backup and selective restore commands were checked against a scratch volume. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: re-apply custom Tomcat XML edits rather than restoring old files Addresses review on #942: covers operators who hand-edited server.xml / web.xml, alongside the existing backup and selective keystore / properties restore. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: create the conf backup archive mode 600 Addresses review on #942: the archive holds the TLS private key and the database password, but under the default 022 umask it was created 0644. The backup now runs with umask 077 and chmods the archive to 600 before writing (which also restricts an existing archive being overwritten). The docs say to delete it after the upgrade, and that it is root-owned on Linux. Checked against a scratch volume, overwriting an existing 0644 file. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: Spring is a runtime dependency for SAML SSO, not test-only Addresses review on #942: AGENTS.md said Spring 7 was used only in tests, but pac4j-saml needs it at runtime and SamlSso uses spring-core's ByteArrayResource. Separates the runtime and test uses, and warns against test-scoping spring-core. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: start the upgraded stack detached before restoring conf files Addresses review on #942: an attached docker-compose up has to be interrupted to run the restore commands, which stops the databases too, and the later restart of web doesn't bring them back. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: keep the conf backup outside the Docker build context 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> * docs: document the OpenSAML repository controls and PGP verification Explains why OpenSAML comes from Shibboleth's repository, the repository filter, the pinned-key PGP verification, the no-wildcard rule for .mvn/pgp-keys.map, how to add a dependency, and the CI test. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: Maven 3.9.2+ requirement for the Shibboleth repository filter Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: note the hamcrest-core 1.1 noSig exception in AGENTS.md Addresses review on #942: the signature-checking guidance said every dependency is verified, but .mvn/pgp-keys.map allows the unsigned org.hamcrest:hamcrest-core:1.1 (via json-simple, #944) by exact version. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: AGENTS.md notes SSO gating, SP-initiated-only login and blank-ID rejection Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: binding cookie is scoped to the SAML directory Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: the bearer confirmation must name the same SAML request Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: how to run the full CI pipeline locally before pushing Running only mvn test let compile, Docker and integration-test failures reach CI. Document every test.yml step and the things CI does that a local run has to repeat: the challenge DB user host rewrite, restoring the official Mongo image, clearing leftover Mongo containers, and running in a throwaway worktree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: each SAML login has its own binding cookie Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: fix only what a change introduces, file issues for the rest Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Configuration menu - View commit details
-
Copy full SHA for dd4cedb - Browse repository at this point
Copy the full SHA dd4cedbView commit details
This comparison is taking too long to generate.
Unfortunately it looks like we can’t render this comparison for you right now. It might be too big, or there might be something weird with your repository.
You can try running this command locally to see the comparison on your machine:
git diff dev@{1day}...dev