Skip to content

Jdk 25 upgrade - #475

Closed
konstjar wants to merge 20 commits into
qafrom
jdk-25-upgrade
Closed

konstjar wants to merge 20 commits into
qafrom
jdk-25-upgrade

Conversation

@konstjar

Copy link
Copy Markdown
Collaborator

Platform upgrade

  • Java 8 -> 25, Spring Boot 2.3.9 -> 4.1.0;
  • Auth layer rebuilt on native Spring Security; pac4j removed. SAML2 relying party, JWT service + filter (nimbus-jose-jwt; jjwt dropped), API-token filter. 11 pac4j-era classes deleted.
  • springdoc 1.4.8 -> 3.0.3, flyway-database-postgresql, spring-boot-starter-mail, Joda-Time -> java.time

Build & packaging

  • UI now builds from source: ui git submodule + frontend-maven-plugin (pinned Node v24.11.1). arachne-commons (retired) vendored into dto/arachne, contract locked by a test.
  • Unused deps removed (jjwt, jtds, javax.jms-api, hamcrest); production jar no longer ships test-scoped libraries; Jetty BOM pinned so Solr 8.11.4 resolves
  • Testcontainers 1.17.5->1.20.6

Correctness

  • Admin download statistics: fixed a cache miss that broke the whole endpoint when a downloader's account had been deleted, removed unsafe parallel traversal of lazy
  • Hibernate associations, and made sorting null-safe for those rows
  • CSV exports no longer stream partial output or mask the real exception on failure
  • Async executors now drain in-flight work on shutdown (previously a redeploy silently abandoned bundle generation and dropped queued e-mails)
  • logging.path was removed in Boot 3

@gorodonoff
gorodonoff force-pushed the jdk-25-upgrade branch 3 times, most recently from f8c9d64 to 2bfb482 Compare August 1, 2026 19:53
@konstjar
konstjar force-pushed the jdk-25-upgrade branch 2 times, most recently from 4ef974a to bcecfff Compare August 3, 2026 09:07
konstjar added 13 commits August 3, 2026 11:37
The UI arrived as com.odysseusinc.athena:athena-resources, a zip published to a
private Maven repository, and arachne-commons came from the same place. Both made
a clean checkout unbuildable without credentials, and neither repository is
reachable any more.

The front end is now built in the same reactor: AthenaUI is a git submodule
pinned to an exact commit, and frontend-maven-plugin downloads its own Node and
runs `npm ci && npm run build`, so no Node is required on the build host and the
packaged UI is identified by revision rather than by a mutable snapshot. Use
-PskipUi to reuse whatever was last built when working on the back end alone; an
enforcer rule fails the build if a jar would otherwise ship without a UI.

The four DTOs actually used from arachne-commons are vendored under
dto/arachne, with a contract test pinning the JSON the portal returns. The
retired library is no longer a dependency.

Also declares a baseline for ${env}: the dev profile is activeByDefault, and
Maven drops activeByDefault profiles as soon as any -P is given, so
`mvn -PskipUi package` resolved no properties directory and produced a jar with
no application.properties at all.

Version moves to 1.16.0-SNAPSHOT.
src/main/resources/opencsv.jar was a patched build of an unreleased fork,
committed as a binary with a system-scoped dependency pointing into the source
tree. It could not be resolved by anyone else, could not be audited, and calls
sun.misc.Cleaner, which was removed in Java 9 -- so it cannot run on any
supported JDK.

AthenaCSVWriter now writes the format directly and AthenaResultSetHelperService,
which existed only to feed the fork, is replaced by ResultSetCsvValues. The
output is byte-identical to what the jar produced; a golden test pins that so
the claim is checked rather than asserted, and a second test covers the
spreadsheet formula escaping the format relies on.
Java 8 has been out of support for years and blocks everything downstream:
Spring Boot 4 requires 17 or later, and several dependencies carrying known
advisories have no release that still targets 8.

Two compiler settings are needed to get Lombok working on a modern JDK. javac
must be forked, because maven-compiler-plugin 3.14+ runs it in-process and
Lombok's patching then aborts the build; and -proc:full restores classpath
discovery of annotation processors, which JDK 23 stopped doing implicitly --
without it Lombok silently generates nothing and every accessor becomes
"cannot find symbol".

This commit does not build on its own. Spring Boot 2.3 does not support this
JDK, and the pac4j security layer does not compile under the Spring version
that does; the toolchain, framework and security migration only work as a
set. They are kept apart so each can be read on its own. The first commit that
builds again is "feat: replace pac4j with Spring Security".
2.3 went out of even commercial support years ago, so the framework itself no
longer receives fixes, and it does not run on a supported JDK.

Most of the diff is mechanical: Boot 3 moved the Java EE APIs to the jakarta
namespace, so every javax.persistence, javax.validation and javax.servlet
import is rewritten. Those files change by import line only.

The rest are places where the upgrade is not mechanical:

- springdoc moves to the starter-webmvc-ui artifact; the 1.x line has no build
  that works past Boot 2.
- Flyway 10 moved per-database support out of flyway-core, so the PostgreSQL
  module is now an explicit dependency.
- Mail moves to spring-boot-starter-mail, and the standalone jakarta.mail
  dependency goes.
- Jetty is pinned to 9.4 through a BOM. Boot 4 manages Jetty 12, but solrj 8.11
  is compiled against 9.4 and calls a class Jetty 12 moved; unpinned, the tree
  mixes both and every Solr query fails at runtime.
- antlr4-runtime is pinned to 4.13.2. Hibernate 7 needs it for HQL parsing,
  while solr-core drags in 4.5.1-1 and wins on nearest definition, leaving
  every repository bean failing to load.
- logging.path was removed in Boot 3, so no profile had a log location any
  more and logback wrote to a file called LOG_FILE_IS_UNDEFINED. The key is
  renamed in all four profiles, with a default in logback-spring.xml as a
  backstop.

This commit does not build on its own: the pac4j security layer does not
compile against this Spring version. See the following commit.
pac4j 3.3.0 is what blocked the framework upgrade: it has no integration with
Spring Framework 7, it is several major versions behind, and the
spring-security-pac4j bridge stopped being released. Nothing in the project
needed a second security framework layered on top of Spring Security, so it is
gone rather than replaced like for like.

What takes its place, all on Spring Security's own APIs:

- SamlRelyingPartyConfig registers a SAML2 relying party, building the
  registration lazily so a missing keystore or unreachable metadata does not
  stop the application starting. The assertion consumer service stays on
  /auth/callback so the identity provider needs no change.
- JwtTokenService issues and verifies tokens on nimbus-jose-jwt; jjwt 0.9.1,
  which was last released in 2018, is dropped. JwtAuthenticationFilter reads
  them off the configured header.
- ApiTokenAuthenticationFilter and the existing HMAC filter cover the
  server-to-server endpoints.
- AthenaAuthentication carries the authenticated user in place of the pac4j
  profile, so AthenaProfile and UserProfileUtil are no longer needed.

The endpoints that are reachable without authentication are now a single
declared list rather than a set of ignore rules spread across four adapters,
and PublicEndpointMatchingTest asserts that list -- including which paths must
stay behind authentication. The filters have unit tests, which the pac4j
configuration never had.

Also removes src/main/resources/saml/samlKeystore.jks. Key material does not
belong in the repository; it is supplied at deployment time.

This is the commit at which the branch compiles again, completing the group
started by the JDK and Spring commits.
Version bumps, each for a published advisory or because the pinned version has
no build that works on the current framework:

  commons-lang3     3.11      -> 3.18.0
  postgresql        42.2.26   -> 42.7.7
  guava             30.1-jre  -> 33.4.8-jre
  solr / solrj      8.11.2    -> 8.11.4
  metrics-core      3.2.0     -> 4.2.39
  org.json          20190722  -> 20260719
  commons-beanutils           -> 1.11.0 (pinned; arrived transitively)
  lombok            1.18.18   -> 1.18.46 (earlier builds do not run on JDK 25)

Testcontainers moves to 1.20.6. The pinned 1.17.5 carries docker-java 3.2.x,
which negotiates Docker API 1.32 -- below the floor of every engine since
Docker 25 -- so it fails with HTTP 400 and took the entire suite down with it.
docker.api.version makes the negotiated version explicit.

Dependencies removed because nothing referenced them: jtds (a SQL Server driver
in a project that only talks to PostgreSQL) and hamcrest-core, which shadowed
the version the test framework brings. The guava property was also misspelled
"guava.verson", so the intended version had never actually applied.
The dev profile carried a complete server-to-server client definition,
including the private half of its signing keypair, and a working token salt.
Anyone with a copy of the repository had them.

Both are removed. The profile now documents how to configure a client with a
public key you generate yourself, and the salt is a placeholder that is
obviously not a secret. The keys must remain present as empty values because
the configuration has no defaults for them.

The keypair should be treated as compromised anywhere it was reused; it
remains in this repository's history.

Also gives the test profile a fixed non-secret salt. JwtTokenService requires
at least 32 bytes, so the blank placeholder the deployed profiles carry would
otherwise fail the Spring context.
Separate from the migration, these are behaviours that were wrong before it and
are still wrong after it.

- The admin download statistics failed outright whenever a downloader's account
  had been deleted, and walked lazily-loaded Hibernate associations from a
  parallel stream, which is not safe on a session bound to one thread. Sorting
  is now null-tolerant, since the user, e-mail and organization columns are
  legitimately empty for those rows.
- Two N+1 query patterns: a fetch join for the statistics query, and @batchsize
  where three eager collections prevented a join fetch outright.
- CSV export no longer streams a partial file and then reports success, and no
  longer replaces the underlying failure with a generic message.
- The async executors now drain in-flight work on shutdown. A redeploy
  previously abandoned bundle generation midway and dropped queued mail.
- Solr filter values and facet JSON are escaped rather than concatenated, so a
  query containing the syntax characters no longer changes the query's meaning.
- Joda-Time is replaced by java.time.

Each behaviour has a test; the Solr and CPT4 fixtures the suite needs are added
alongside them.
Rows created before the token column existed carry the empty-string default,
and the token is what makes an accept-by-mail link specific to one licence, so
those licences could never be accepted from an e-mail.

The migration backfills them, drops the default so an insert that omits the
token fails instead of quietly storing an empty one, and adds a check
constraint. Tokens are generated with gen_random_uuid() rather than
md5(random()), whose output is predictable from other values drawn in the same
session, and which the application does not use either.

Blank is treated as empty on both sides: the constraint trims before
comparing, and the service already rejects a blank token before looking the row
up, so a whitespace token would be as useless as an empty one.
The file described search ranking in detail and almost nothing else: how to
build the project, what it needs to run, or how the pieces fit together were
all absent, and some of what was there no longer matched the code.

It now opens with what the service is and what it runs on, then covers
prerequisites, building, configuration and tests, and keeps the search
behaviour and query-tuning reference after that. The searched-word counts in
the ranking examples are corrected -- they were labelled 3/2/1 for rows
matching five, four and three words.
The repository had no continuous integration at all, so nothing checked that a
change compiled, let alone that the suite passed, until someone built it by
hand.

The workflow builds on JDK 25 and runs the full suite. It also asserts the
working tree is clean afterwards: several tests write into the source tree, and
that only shows up as an unexplained dirty checkout later.
Four defects that surfaced once the application moved to Spring Security's own
SAML support:

- The login popup stopped on an empty provider chooser. Spring Security
  auto-redirects to a lone identity provider only when it can iterate the
  registration repository, and ours deliberately cannot -- iterating it would
  force the credential and metadata load at startup that the lazy repository
  exists to defer. The entry point is pinned explicitly instead.
- AuthnRequests were not signed unless the identity provider's metadata asked
  for it, while our own metadata declares AuthnRequestsSigned. Sign
  unconditionally so the two sides cannot disagree.
- A second login attempt reached /auth/sso with the first one's session still
  authenticated. Nothing maps that path, so it ended in
  NoResourceFoundException; re-run the success handler and issue a token.
- Logout left the local session intact, so the next login could re-issue a
  token for the previous user with no credentials presented -- visible as being
  unable to sign in as somebody else. Drop the session, then redirect to the
  identity provider with an encoded service parameter so the popup comes back
  to a landing page that closes it.

The landing URL is derived from the request, with athena.url only as an
override, so the flow needs no environment-specific address configured.

Also corrects the public endpoint list: the About dialog reads
/api/v1/vocabularies/release-version before sign-in, and the entry spelled it
in camelCase, which matches no handler. The server-to-server controller
inherits the same handler and stays behind signature verification.
The packaged profiles carried the hostnames of real deployments, a database
password and a token secret, all of it in a public repository and in a
pullable image.

Every such value in the qa and prod profiles is now read from the environment
as ${VAR:}, and README.md lists what has to be set. The dev and test profiles
keep working values, but example.org ones -- they are fixtures, not
deployments, and nothing outside them has to exist for the suite to run.

Identity provider metadata and the signing keystore are no longer expected on
the classpath at all. Both are per-environment data with a lifecycle of their
own; they are mounted at run time and pointed at by CAS_IDP_METADATA_LOCATION
and CAS_KEYSTORE_FILE. /auth/saml-metadata answers 404 when no copy is
packaged rather than failing -- nothing in the login flow goes through it.

Removes src/test/resources/test-old.properties, which nothing referenced.

Note for deployers: values previously baked in must now be present in the
environment, or the features that use them will not work. The service provider
metadata registered with the identity provider must also declare an assertion
consumer service resolving to exactly <base>/auth/callback -- metadata
generated for the previous implementation carries a ?client_name=SAML2Client
suffix and has to be regenerated, or assertions are rejected and the browser
appears to loop on login.
konstjar added 7 commits August 3, 2026 12:00
.gitmodules declared the submodule but no gitlink was ever recorded, so
`git submodule update --init` did nothing on a fresh clone and `mvn package`
had no front end to build -- the one part of building the UI from source that
was not actually true end to end.

Pinned at the AthenaUI commit this back end requires, not the current release.
The two are coupled: the token now arrives in the URL fragment rather than the
query string, and error responses carry real HTTP status codes instead of 200
for everything. An older front end does not read either.
Logging out did not invalidate anything. The logout handler took the token from
the security context, but the authentication filter was inserted before
BasicAuthenticationFilter, which sits far later in the chain than LogoutFilter.
On a STATELESS chain the context was therefore still empty when the handler ran,
so revoke() was never reached -- while logout still answered 200. The token
remained valid for its full seven-day lifetime.

This is a regression against the pac4j CustomLogoutLogic it replaced, which read
the token out of the session profile and did invalidate it. /auth/slo does not
cover it either: that clears the HTTP session and never touches the revocation
store.

Two changes, because either alone leaves the trap in place:

- The filter is now inserted before LogoutFilter, so the context is populated
  for the whole request.
- The handler no longer depends on that being true. It falls back to the token
  on the request, verifying it first -- the header is unauthenticated input and
  the revocation store is unbounded, so an unverified value must not reach it.

The tests drive the handler with an empty context on purpose, since that is the
state the bug produced. Reverting the handler to the context-only version fails
revokesTheTokenWhenTheContextIsEmpty, which is the point.
Three small defects on the server-to-server path, none of which changes what a
correctly formed request does.

- The bearer prefix carried no trailing space, so "Bearer" run together with a
  token still parsed: the value was taken from offset six regardless. A header
  of that shape is not the scheme and is now refused.

- HmacVerifyingFilter is a Filter bean, and Spring Boot auto-registers those
  with the servlet container, so it ran on every request ahead of all four
  security chains. OncePerRequestFilter made the in-chain instance a no-op
  afterwards, which is why nothing misbehaved visibly. It now gets the same
  disabled FilterRegistrationBean as the other two filters, which the comment
  above them already explained was necessary.

- verify() caught only BadCredentialsException, but an unrecognised client id
  arrives from ApiClients as AuthenticationCredentialsNotFoundException. That
  escaped as a 500 rather than a refusal, and since the filter inspects every
  request carrying the header, the fault was not confined to /api/s2s/**.
Only the signing half of the keypair was registered. The pac4j SAML2Client this
replaced used the same keystore for decryption too, so an identity provider that
encrypts assertions or NameIDs -- which is the default in several products --
would have had its response rejected.

Nothing would have surfaced it before release: the registration is built lazily
on purpose, so the gap only appears when somebody actually authenticates.

The keystore is now read once and both credentials derive from it.
athena.token.secret is read by nothing. Tokens are signed with `salt`, and have
been since the JWT service replaced pac4j's generator. Leaving the key in all
four profiles invites someone to rotate it and conclude, correctly but
uselessly, that it had no effect.
Ten files still imported org.apache.commons.lang.StringUtils. commons-lang 2 has
been end-of-life since 2011 and was not even a declared dependency -- it arrived
transitively, so the version in the build was whatever some other library
happened to pull in, while commons-lang3 sat alongside it at a pinned 3.18.0.

Every method used here (isBlank, isNotBlank, isEmpty, isNotEmpty, EMPTY,
endsWith, substringBetween, removeEnd, remove, equalsIgnoreCase, countMatches,
split, join) behaves identically in lang3. isNumeric, which does differ -- lang2
returns true for the empty string and lang3 false -- was already imported from
lang3 and is untouched.
Three places where the text no longer matched what the code does.

- The pom described spring-security-saml2-service-provider as resolving OpenSAML
  3.4.6 under Spring Security 5.7. That was true of an intermediate step, not of
  this branch: on Security 7 it resolves OpenSAML 5, which a comment a hundred
  lines further down already said. Two contradictory notes in one file are worse
  than neither.

- The deployed profiles claimed every empty default "surfaces as a clear failure
  in the feature that needs it rather than as a startup crash". SALT is exactly
  a startup crash -- deliberately, since a signing key that is absent or under 32
  bytes cannot be worked around later -- so the blanket statement was wrong for
  the one value where it mattered most.

- The package job was non-blocking because the ui submodule had no committed
  gitlink. It has one now, so continue-on-error goes and a build that cannot
  produce a jar fails the pull request.
@konstjar konstjar closed this Aug 28, 2026
@konstjar
konstjar deleted the jdk-25-upgrade branch August 28, 2026 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant