Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 38 additions & 11 deletions lib/Horde/Core/Factory/Mail.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,21 @@ public function getConfig()

/* Add username/password options now, regardless of current value of
* 'auth'. Will remove in create() if final config doesn't require
* authentication. Need isAuthenticated() check since we may be
* running from CLI with 'user_admin' registry flag, which sets
* the authentication name but not the credentials. */
* authentication. Need the isAuthenticated() check since we may be
* running from CLI with the 'user_admin' registry flag. That flag
* sets the authentication name but not the credentials.
*
* password_auth means "use the logged-in user's password for SMTP".
* CLI tools like horde-alarms authenticate by name only. They have
* no session password. So getAuthCredential('password') comes back
* empty. When password_auth wants that missing password, we derive
* neither the username nor the password and keep both master values.
* Pairing the auth username with the master password would mismatch
* the SMTP account. This keeps the fix in the mail factory. It does
* not fabricate a session. See the review on horde/Core#219.
*
* Problem originally reported and fixed by Torben Dannhauer
* <torben@dannhauer.de> in horde/Core#219. */
if (strcasecmp($transport, 'smtp') === 0) {
if ($registry->isAuthenticated()
&& strlen((string) ($auth = $registry->getAuth()))) {
Expand All @@ -116,26 +128,41 @@ public function getConfig()
}
// Don't set password when using XOAUTH2
} else {
// Hook returned regular credentials
// Hook returned regular credentials. Resolve the
// session password once. When password_auth wants
// it but the session has none, derive neither field
// and keep both master values. Pairing the auth
// username with the master password would mismatch
// the SMTP account.
$cred = $registry->getAuthCredential('password');
$hasCred = strlen((string) $cred) > 0;

if (isset($smtp_creds['username'])) {
$params['username'] = $smtp_creds['username'];
} elseif (!empty($params['username_auth'])) {
} elseif (!empty($params['username_auth'])
&& (empty($params['password_auth']) || $hasCred)) {
$params['username'] = $auth;
}

if (isset($smtp_creds['password'])) {
$params['password'] = $smtp_creds['password'];
} elseif (!empty($params['password_auth'])) {
$params['password'] = $registry->getAuthCredential('password');
} elseif (!empty($params['password_auth']) && $hasCred) {
$params['password'] = $cred;
}
}
} catch (Horde_Exception_HookNotSet $e) {
// No hook defined, use default username/password
if (!empty($params['username_auth'])) {
// No hook defined, use default username/password. Same
// pairing rule as above: skip the auth username when
// password_auth wants a session password we don't have.
$cred = $registry->getAuthCredential('password');
$hasCred = strlen((string) $cred) > 0;

if (!empty($params['username_auth'])
&& (empty($params['password_auth']) || $hasCred)) {
$params['username'] = $auth;
}
if (!empty($params['password_auth'])) {
$params['password'] = $registry->getAuthCredential('password');
if (!empty($params['password_auth']) && $hasCred) {
$params['password'] = $cred;
}
}
}
Expand Down
185 changes: 185 additions & 0 deletions test/Unit/Factory/MailFactoryTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
<?php

declare(strict_types=1);

/**
* Copyright 2026 The Horde Project (http://www.horde.org/)
*
* See the enclosed file LICENSE for license information (LGPL). If you
* did not receive this file, see http://www.horde.org/licenses/lgpl21.
*
* @author Torben Dannhauer <torben@dannhauer.de>
* @category Horde
* @copyright 2026 The Horde Project
* @license http://www.horde.org/licenses/lgpl21 LGPL 2.1
* @package Core
*/

namespace Horde\Core\Test\Unit\Factory;

use Horde_Core_Factory_Mail;
use Horde_Core_Hooks;
use Horde_Exception_HookNotSet;
use Horde_Injector;
use Horde_Injector_TopLevel;
use Horde_Registry;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\Attributes\Test;
use PHPUnit\Framework\TestCase;

/**
* Ensures SMTP username_auth / password_auth fall back to configured master
* credentials when the session has an auth name but no password — the
* horde-alarms CLI user_admin case that previously sent empty SMTP passwords.
*/
#[CoversClass(Horde_Core_Factory_Mail::class)]
class MailFactoryTest extends TestCase
{
private array $confBackup = [];
private $registryBackup;

protected function setUp(): void
{
$this->confBackup = $GLOBALS['conf'] ?? [];
$this->registryBackup = $GLOBALS['registry'] ?? null;
}

protected function tearDown(): void
{
$GLOBALS['conf'] = $this->confBackup;
if ($this->registryBackup === null) {
unset($GLOBALS['registry']);
} else {
$GLOBALS['registry'] = $this->registryBackup;
}
}

#[Test]
public function testCliUserAdminKeepsMasterSmtpCredentials(): void
{
$this->configureMailer([
'username' => 'smtp-master@example.com',
'password' => 'master-secret',
'username_auth' => true,
'password_auth' => true,
'auth' => true,
]);

$registry = $this->createMock(Horde_Registry::class);
$registry->method('isAuthenticated')->willReturn(true);
$registry->method('getAuth')->willReturn('admin@example.com');
$registry->method('getAuthCredential')
->with('password')
->willReturn(false);
$GLOBALS['registry'] = $registry;

[$transport, $params] = $this->factoryWithoutSmtpHook()->getConfig();

$this->assertSame('smtp', $transport);
$this->assertSame('smtp-master@example.com', $params['username']);
$this->assertSame('master-secret', $params['password']);
$this->assertArrayNotHasKey('username_auth', $params);
$this->assertArrayNotHasKey('password_auth', $params);
}

#[Test]
public function testSessionCredentialsReplaceMasterWhenPresent(): void
{
$this->configureMailer([
'username' => 'smtp-master@example.com',
'password' => 'master-secret',
'username_auth' => true,
'password_auth' => true,
'auth' => true,
]);

$registry = $this->createMock(Horde_Registry::class);
$registry->method('isAuthenticated')->willReturn(true);
$registry->method('getAuth')->willReturn('alice@example.com');
$registry->method('getAuthCredential')
->with('password')
->willReturn('alice-secret');
$GLOBALS['registry'] = $registry;

[, $params] = $this->factoryWithoutSmtpHook()->getConfig();

$this->assertSame('alice@example.com', $params['username']);
$this->assertSame('alice-secret', $params['password']);
}

#[Test]
public function testPasswordAuthAloneKeepsMasterUsername(): void
{
$this->configureMailer([
'username' => 'smtp-master@example.com',
'password' => 'master-secret',
'username_auth' => false,
'password_auth' => true,
'auth' => true,
]);

$registry = $this->createMock(Horde_Registry::class);
$registry->method('isAuthenticated')->willReturn(true);
$registry->method('getAuth')->willReturn('alice@example.com');
$registry->method('getAuthCredential')
->with('password')
->willReturn('alice-secret');
$GLOBALS['registry'] = $registry;

[, $params] = $this->factoryWithoutSmtpHook()->getConfig();

$this->assertSame('smtp-master@example.com', $params['username']);
$this->assertSame('alice-secret', $params['password']);
}

#[Test]
public function testEmptySessionPasswordDoesNotClearMasterPassword(): void
{
$this->configureMailer([
'username' => 'smtp-master@example.com',
'password' => 'master-secret',
'username_auth' => true,
'password_auth' => true,
'auth' => true,
]);

$registry = $this->createMock(Horde_Registry::class);
$registry->method('isAuthenticated')->willReturn(true);
$registry->method('getAuth')->willReturn('admin@example.com');
$registry->method('getAuthCredential')
->with('password')
->willReturn('');
$GLOBALS['registry'] = $registry;

[, $params] = $this->factoryWithoutSmtpHook()->getConfig();

$this->assertSame('smtp-master@example.com', $params['username']);
$this->assertSame('master-secret', $params['password']);
}

/**
* @param array<string, mixed> $params
*/
private function configureMailer(array $params): void
{
$GLOBALS['conf'] = [
'mailer' => [
'type' => 'smtp',
'params' => $params,
],
];
}

private function factoryWithoutSmtpHook(): Horde_Core_Factory_Mail
{
$hooks = $this->createMock(Horde_Core_Hooks::class);
$hooks->method('callHook')
->with('smtp_credentials', 'horde', $this->anything())
->willThrowException(new Horde_Exception_HookNotSet());

$injector = new Horde_Injector(new Horde_Injector_TopLevel());
$injector->setInstance('Horde_Core_Hooks', $hooks);

return new Horde_Core_Factory_Mail($injector);
}
}
Loading