#345 Fix registration of CES password-management service (regression of #163) by henry-bobka · Pull Request #349 · cloudogu/cas

henry-bobka · 2026-07-02T18:50:19Z

After the CAS 6 => 7 / Spring Boot 2 => 3 upgrade, no mails were sent for CAS-driven flows but the UI reported "Password Reset Instructions Sent Successfully" — so the failure was invisible.

The drop was address-dependent and on the application layer:

admin@ces.local => CAS logs Email address [admin@ces.local] for [admin] is not valid and sends nothing.
admin@example.com => mail flows normally.

This is a regression of #163: CesLdapPasswordManagementService (which deliberately deactivates the e-mail-address validation for internal addresses like admin@ces.local) was no longer the active passwordChangeService at runtime.

Resolve #345

ppxl

Thanks for the PR 💙 That one assertion simplification aside, I think the only point I would like to discuss is the matter whether we should replace Java classes with updated ones for security reasons. Are there special reasons to do so?

If I extrapolate this path, we would be solely patching classes in libaries from libaries from libaries, leaving us in patch hell. That is quite tedious because in my book this is supposed to be fixed on the side of the actual library (this case being Apereo CAS).

ppxl · 2026-07-07T08:23:05Z

+                    "WEB-INF/lib/jaxws-rt-*.jar",
+                    // Trivy CRITICAL fixes: drop the base WAR's vulnerable jars, keeping the patched
+                    // versions from this overlay's classpath (see gradle.properties / build.gradle).
+                    // CVE-2026-22732
+                    "WEB-INF/lib/spring-security-config-*.jar",
+                    "WEB-INF/lib/spring-security-core-*.jar",
+                    "WEB-INF/lib/spring-security-crypto-*.jar",
+                    "WEB-INF/lib/spring-security-web-*.jar",
+                    // CVE-2026-40477/40478/41901
+                    "WEB-INF/lib/thymeleaf-3*.jar",
+                    "WEB-INF/lib/thymeleaf-spring6-*.jar",
+                    // CVE-2025-14813
+                    "WEB-INF/lib/bcprov-jdk18on-*.jar",
+                    "WEB-INF/lib/bcpkix-jdk18on-*.jar",
+                    "WEB-INF/lib/bcutil-jdk18on-*.jar"


I don't grasp why we have to replace the jars by ourselves.

Won't this turn into a maintenance nightmare if we start here to singlehandedly replace single parts? I expected to see this as the work of the vendor.

You're right in principle. Long-term this should belong upstream. A clean fix would be a CAS-release that bumps these deps.

The reason it is done here: these are Trivy CRITICAL findings inside the cas dogu. Waiting for Apereo means shipping a vulnerable auth component until they publish an update.

I'll treat this as a temporary workaround. I'd suggest to open up a follow-up issue to remove it until a patched CAS version is available.

ppxl · 2026-07-07T08:27:25Z

+     * Regression guard for #345 / #163: the config must not be gated by the indexed
+     * {@code @ConditionalOnProperty("cas.authn.pm.ldap[0].ldap-url")}, which stopped resolving
+     * after the CAS 6->7 / Spring Boot 2->3 upgrade and silently deactivated the CES override.


That's some heavy lifting of a comment. Would this comment also be put to the line which was removed? (Except the Regression guard, that fits perfectly and I love the idea 💙

Done. Moved the "why the indexed @ConditionalOnProperty was removed" comment to the class Javadoc of LdapPasswordManagementConfiguration. Kept the test comment.

ppxl · 2026-07-07T09:07:23Z

One other thing I would like to discuss is the way we write commit messages. I see your git commit message headers tend to get quite long some 100+ characters long (f. i. 8daf3f3) and I feel these could be shorter.

Suggested read to this is Chris Beams's post on a great git commit message:

https://chris.beams.io/git-commit#seven-rules

We see the commit message a medium to transport useful information that exceeds the actual change. Like an email, the git message can be seen as a subject which shortly adresses the matter of the change. Along follows a body where details can be presented. Especially of why the change is necessary. Usually that means describing a transition of

how it was working before to
(possibly) why that is now a problem over to
how it is supposed to work now

The subject in this case shouldn't describe in all detail here because that is just the mental grip to the actual matter.

For the CES team, we almost take these rules verbatim with two exceptions:

We allow also a commit message header to 72 chars
We have an issue ID in the header (you already do this perfectly)
- here we have an exception to the exception: the Github issue / ticket system number can be left out when it comes to mundane boyscouting changes when these don't justify their own issues/tickets
- if not sure commit under the ID you already have at hand.

Also I would like to encourage you to give the git commit message body a chance. This matter allows to add meta data on your change where a code comment might feel not at place. Sometimes these message bodies can be very long while sometimes they stay empty.

After the CAS 6->7 / Spring Boot 2->3 upgrade the CES override was no longer the active passwordChangeService, so Apereo's stock LDAP service ran e-mail validation and silently dropped reset mails for internal addresses like admin@ces.local (regression of #163). Register the CES service under both CAS bean names so the core and LDAP auto-configs back off, and drop the indexed @ConditionalOnProperty that stopped resolving under Spring Boot 3.

…service_registration

The base CAS webapp WAR bundles vulnerable spring-security, thymeleaf and bouncycastle jars. Exclude them from the overlay and keep patched versions on the classpath: - CVE-2026-22732: spring-security 6.5.5 -> 6.5.9 - CVE-2026-40477/40478/41901: thymeleaf 3.1.3 -> 3.1.5.RELEASE - CVE-2025-14813: bouncycastle 1.82 -> 1.84

henry-bobka · 2026-07-07T13:29:48Z

One other thing I would like to discuss is the way we write commit messages. I see your git commit message headers tend to get quite long some 100+ characters long (f. i. 8daf3f3) and I feel these could be shorter.

Suggested read to this is Chris Beams's post on a great git commit message:

https://chris.beams.io/git-commit#seven-rules

We see the commit message a medium to transport useful information that exceeds the actual change. Like an email, the git message can be seen as a subject which shortly adresses the matter of the change. Along follows a body where details can be presented. Especially of why the change is necessary. Usually that means describing a transition of

how it was working before to

(possibly) why that is now a problem over to

how it is supposed to work now

The subject in this case shouldn't describe in all detail here because that is just the mental grip to the actual matter.

For the CES team, we almost take these rules verbatim with two exceptions:

We allow also a commit message header to 72 chars

We have an issue ID in the header (you already do this perfectly)

here we have an exception to the exception: the Github issue / ticket system number can be left out when it comes to mundane boyscouting changes when these don't justify their own issues/tickets

if not sure commit under the ID you already have at hand.

Also I would like to encourage you to give the git commit message body a chance. This matter allows to add meta data on your change where a code comment might feel not at place. Sometimes these message bodies can be very long while sometimes they stay empty.

Thanks for this detailed insight 👍 This makes sense - I've reworked the history: subjects are now <=72 chars and details moved into the commit bodies.

ppxl · 2026-07-09T13:18:52Z

For the Dockerfile: There is a new official/base image version available: 3.23.4-2 -> 3.24.1-1

ppxl

Very nice, it works well. There is this one comment about the updated base image in Dockerfile which should be updated as well.

Please also update docs/gui/release_notes_*.md

henry-bobka · 2026-07-09T18:13:47Z

For the Dockerfile: There is a new official/base image version available: 3.23.4-2 -> 3.24.1-1

Done. I adjusted the version.

henry-bobka · 2026-07-09T18:14:24Z

Very nice, it works well. There is this one comment about the updated base image in Dockerfile which should be updated as well.

Please also update docs/gui/release_notes_*.md

Thanks 👍 I also updated the release notes.

ppxl

LGTM 🚀

henry-bobka mentioned this pull request Jul 2, 2026

#345 Fix registration of CES password-management service (regression of #163) #346

Closed

ppxl requested changes Jul 7, 2026

View reviewed changes

henry-bobka added 9 commits July 7, 2026 14:50

#345 Add regression tests for CES password service

1c6e433

#345 Add changelog entry

cbaaa46

Merge remote-tracking branch 'origin/develop' into bugfix/345_fix_pm_…

e4b7063

…service_registration

#345 Move bean-wiring rationale to @bean declaration

748b2c8

#345 Use assertInstanceOf in password service test

2f45b74

#345 Move CVE fixes to changelog Security section

7736399

#345 Move ConditionalOnProperty rationale to config class

df43d24

henry-bobka force-pushed the bugfix/345_fix_pm_service_registration branch from 8daf3f3 to df43d24 Compare July 7, 2026 13:09

ppxl requested changes Jul 9, 2026

View reviewed changes

henry-bobka added 2 commits July 9, 2026 20:05

#345 Bump base image to 3.24.1-1

ea715f6

#345 Update release notes

271240a

ppxl approved these changes Jul 10, 2026

View reviewed changes

ppxl merged commit 1aa0827 into develop Jul 10, 2026
1 check passed

ppxl deleted the bugfix/345_fix_pm_service_registration branch July 10, 2026 05:50

Uh oh!

Conversation

henry-bobka commented Jul 2, 2026 • edited by ppxl Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ppxl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ppxl Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

henry-bobka Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

ppxl Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

henry-bobka Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

ppxl commented Jul 7, 2026

Uh oh!

henry-bobka commented Jul 7, 2026

Uh oh!

ppxl commented Jul 9, 2026

Uh oh!

ppxl left a comment

Choose a reason for hiding this comment

Uh oh!

henry-bobka commented Jul 9, 2026

Uh oh!

henry-bobka commented Jul 9, 2026

Uh oh!

ppxl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

henry-bobka commented Jul 2, 2026 •

edited by ppxl

Loading