Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: OWASP/SecurityShepherd
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: dev@{1day}
Choose a base ref
...
head repository: OWASP/SecurityShepherd
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: dev
Choose a head ref
  • 7 commits
  • 202 files changed
  • 4 contributors

Commits on Oct 23, 2026

  1. 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>
    ismisepaul and claude authored Oct 23, 2026
    Configuration menu
    Copy the full SHA
    34847fe View commit details
    Browse the repository at this point in the history
  2. 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>
    4 people authored Oct 23, 2026
    Configuration menu
    Copy the full SHA
    0558c59 View commit details
    Browse the repository at this point in the history
  3. 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>
    ismisepaul and claude authored Oct 23, 2026
    Configuration menu
    Copy the full SHA
    ef571f7 View commit details
    Browse the repository at this point in the history
  4. 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>
    ismisepaul and claude authored Oct 23, 2026
    Configuration menu
    Copy the full SHA
    c462ad7 View commit details
    Browse the repository at this point in the history
  5. 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>
    ismisepaul and claude authored Oct 23, 2026
    Configuration menu
    Copy the full SHA
    cb548ee View commit details
    Browse the repository at this point in the history
  6. 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>
    ismisepaul and claude authored Oct 23, 2026
    Configuration menu
    Copy the full SHA
    7f26fec View commit details
    Browse the repository at this point in the history
  7. 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>
    ismisepaul and claude authored Oct 23, 2026
    Configuration menu
    Copy the full SHA
    dd4cedb View commit details
    Browse the repository at this point in the history
Loading