Skip to content

fix(mail): keep master SMTP credentials for CLI without session password - #219

Merged
ralflang merged 1 commit into
FRAMEWORK_6_0from
fix/mail-cli-smtp-credentials
Aug 26, 2026
Merged

fix(mail): keep master SMTP credentials for CLI without session password#219
ralflang merged 1 commit into
FRAMEWORK_6_0from
fix/mail-cli-smtp-credentials

Conversation

@TDannhauer

Copy link
Copy Markdown
Contributor

Summary

  • Keep configured master SMTP username/password when CLI/user_admin has an auth name but no session password
  • Prevents horde-alarms from authenticating to SMTP with an empty password under username_auth/password_auth
  • Add unit coverage for CLI fallback and normal session credential overlay

Motivation

horde-alarms initializes Horde with user_admin, which sets an admin identity without credentials. With mailer username_auth/password_auth enabled, Horde_Core_Factory_Mail::getConfig() overwrote the configured master SMTP credentials with that admin username and getAuthCredential('password') (empty/false). SMTP then returned Server denied authentication. Because failed alarm mails never set internal['mail']['sent'], the same alarm was retried on every cron run until a web session with real credentials sent it successfully — producing intermittent multi-hour error bursts.

Changes

  • Apply session SMTP credentials only when a non-empty session password is present
  • When password_auth is enabled but no session password exists, retain master username and password
  • Partial smtp_credentials hook results only overlay the omitted keys
  • New MailFactoryTest covering CLI fallback, session overlay, and empty-password handling

Test plan

  • vendor/bin/phpunit -c vendor/horde/core/phpunit.xml.dist --bootstrap vendor/autoload.php vendor/horde/core/test/Unit/Factory/MailFactoryTest.php
  • Run horde-alarms while a mail alarm is pending; confirm SMTP auth succeeds with master credentials
  • Confirm a normal web session still sends mail using the logged-in user's credentials when *_auth is enabled

@ralflang

Copy link
Copy Markdown
Member

I don't quite agree with this approach.
The architectural sin is relying on the session in a local CLI context. If you want to fall back to a global master SMTP session this should just be read from the configuration rather than this elaborate fudging into the session where it doesn't belong in the first place.

Co-authored-by: Torben Dannhauer <torben@dannhauer.de>
@ralflang
ralflang force-pushed the fix/mail-cli-smtp-credentials branch from 5f7586b to ae76e39 Compare August 26, 2026 16:34
@ralflang

Copy link
Copy Markdown
Member

I have simplified your PR a bit to reflect my earlier doubts about the complexities introduced.

@ralflang
ralflang merged commit 029fa6f into FRAMEWORK_6_0 Aug 26, 2026
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.

2 participants