Skip to content

Commit c7a1724

Browse files
kesselbkesselb
committed
fix: use alias name for from header
Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
1 parent b5db165 commit c7a1724

File tree

2 files changed

+107
-20
lines changed

2 files changed

+107
-20
lines changed

‎lib/Service/MailTransmission.php

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -76,14 +76,23 @@ public function sendMessage(Account $account, LocalMessage $localMessage): void
7676
$bcc = $this->transmissionService->getAddressList($localMessage, Recipient::TYPE_BCC);
7777
$attachments = $this->transmissionService->getAttachments($localMessage);
7878

79-
$alias = null;
79+
$name = $account->getName();
80+
$emailAddress = $account->getEMailAddress();
81+
8082
if ($localMessage->getAliasId() !== null) {
81-
$alias = $this->aliasesService->find($localMessage->getAliasId(), $account->getUserId());
83+
try {
84+
$alias = $this->aliasesService->find($localMessage->getAliasId(), $account->getUserId());
85+
$name = ($alias->getName() ?? $name);
86+
$emailAddress = $alias->getAlias();
87+
} catch (DoesNotExistException) {
88+
$this->logger->debug('The assigned alias no longer exists. Falling back to the default name and email address. It is likely that the alias was deleted or deprovisioned in the meantime.', [
89+
'aliasId' => $localMessage->getAliasId(),
90+
'accountId' => $account->getId(),
91+
]);
92+
}
8293
}
83-
$fromEmail = $alias ? $alias->getAlias() : $account->getEMailAddress();
84-
$from = new AddressList([
85-
Address::fromRaw($account->getName(), $fromEmail),
86-
]);
94+
95+
$from = Address::fromRaw($name, $emailAddress);
8796

8897
$attachmentParts = [];
8998
foreach ($attachments as $attachment) {
@@ -96,7 +105,7 @@ public function sendMessage(Account $account, LocalMessage $localMessage): void
96105
$transport = $this->smtpClientFactory->create($account);
97106
// build mime body
98107
$headers = [
99-
'From' => $from->first()->toHorde(),
108+
'From' => $from->toHorde(),
100109
'To' => $to->toHorde(),
101110
'Cc' => $cc->toHorde(),
102111
'Bcc' => $bcc->toHorde(),
@@ -112,7 +121,7 @@ public function sendMessage(Account $account, LocalMessage $localMessage): void
112121
}
113122

114123
if ($localMessage->getRequestMdn()) {
115-
$headers[Horde_Mime_Mdn::MDN_HEADER] = $from->first()->toHorde();
124+
$headers[Horde_Mime_Mdn::MDN_HEADER] = $from->toHorde();
116125
}
117126

118127
$mail = new Horde_Mime_Mail();

‎tests/Unit/Service/MailTransmissionTest.php

Lines changed: 90 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
use OCA\Mail\Service\TransmissionService;
3333
use OCA\Mail\SMTP\SmtpClientFactory;
3434
use OCA\Mail\Support\PerformanceLogger;
35+
use OCP\AppFramework\Db\DoesNotExistException;
3536
use OCP\EventDispatcher\IEventDispatcher;
3637
use PHPUnit\Framework\MockObject\MockObject;
3738
use Psr\Log\LoggerInterface;
@@ -133,32 +134,67 @@ public function testSendNewMessageSmimeError() {
133134
}
134135

135136
public function testSendMessageFromAlias() {
137+
// Arrange
136138
$mailAccount = new MailAccount();
137-
$mailAccount->setUserId('testuser');
139+
$mailAccount->setName('Bob');
140+
$mailAccount->setEmail('bob@example.org');
141+
$mailAccount->setUserId('bob');
138142
$mailAccount->setSentMailboxId(123);
139-
/** @var Account|MockObject $account */
140-
$account = $this->createMock(Account::class);
141-
$account->method('getMailAccount')->willReturn($mailAccount);
142-
$account->method('getName')->willReturn('Test User');
143-
$account->method('getEMailAddress')->willReturn('test@user');
144-
$account->method('getUserId')->willReturn('testuser');
143+
$account = new Account($mailAccount);
145144
$alias = new Alias();
146145
$alias->setId(1);
147-
$alias->setAlias('a@d.com');
146+
$alias->setName('Info');
147+
$alias->setAlias('info@example.org');
148148
$localMessage = new LocalMessage();
149149
$localMessage->setSubject('Test');
150150
$localMessage->setBodyPlain('Test');
151151
$localMessage->setHtml(false);
152152
$localMessage->setAliasId(1);
153+
$localMessage->setRequestMdn(true);
153154
$transport = $this->createMock(Horde_Mail_Transport::class);
155+
$this->smtpClientFactory->expects($this->once())
156+
->method('create')
157+
->willReturn($transport);
158+
$this->aliasService->expects(self::once())
159+
->method('find')
160+
->willReturn($alias);
161+
$this->transmissionService->expects(self::once())
162+
->method('getSignMimePart')
163+
->willReturnCallback(static fn ($localMessage, $account, $mimePart) => $mimePart);
164+
$this->transmissionService->expects(self::once())
165+
->method('getEncryptMimePart')
166+
->willReturnCallback(static fn ($localMessage, $to, $cc, $bcc, $account, $mimePart) => $mimePart);
167+
168+
// Act
169+
$this->transmission->sendMessage($account, $localMessage);
170+
171+
// Assert
172+
$this->assertEquals(LocalMessage::STATUS_RAW, $localMessage->getStatus());
173+
$this->assertStringContainsString('From: Info <info@example.org', $localMessage->getRaw());
174+
$this->assertStringContainsString('Disposition-Notification-To: Info <info@example.org>', $localMessage->getRaw());
175+
}
154176

177+
public function testSendMessageAliasFallbackName() {
178+
// Arrange
179+
$mailAccount = new MailAccount();
180+
$mailAccount->setName('Bob');
181+
$mailAccount->setEmail('bob@example.org');
182+
$mailAccount->setUserId('bob');
183+
$mailAccount->setSentMailboxId(123);
184+
$account = new Account($mailAccount);
185+
$alias = new Alias();
186+
$alias->setId(1);
187+
$alias->setAlias('info@example.org');
188+
$localMessage = new LocalMessage();
189+
$localMessage->setSubject('Test');
190+
$localMessage->setBodyPlain('Test');
191+
$localMessage->setHtml(false);
192+
$localMessage->setAliasId(1);
193+
$localMessage->setRequestMdn(true);
194+
$transport = $this->createMock(Horde_Mail_Transport::class);
155195
$this->smtpClientFactory->expects($this->once())
156196
->method('create')
157-
->with($account)
158197
->willReturn($transport);
159-
$account->expects($this->once())
160-
->method('getName')
161-
->willReturn('User');
162198
$this->aliasService->expects(self::once())
163199
->method('find')
164200
->willReturn($alias);
@@ -169,8 +205,50 @@ public function testSendMessageFromAlias() {
169205
->method('getEncryptMimePart')
170206
->willReturnCallback(static fn ($localMessage, $to, $cc, $bcc, $account, $mimePart) => $mimePart);
171207

208+
// Act
209+
$this->transmission->sendMessage($account, $localMessage);
210+
211+
// Assert
212+
$this->assertEquals(LocalMessage::STATUS_RAW, $localMessage->getStatus());
213+
$this->assertStringContainsString('From: Bob <info@example.org', $localMessage->getRaw());
214+
$this->assertStringContainsString('Disposition-Notification-To: Bob <info@example.org>', $localMessage->getRaw());
215+
}
216+
217+
public function testSendMessageAliasDoesNotExist() {
218+
// Arrange
219+
$mailAccount = new MailAccount();
220+
$mailAccount->setName('Bob');
221+
$mailAccount->setEmail('bob@example.org');
222+
$mailAccount->setUserId('bob');
223+
$mailAccount->setSentMailboxId(123);
224+
$account = new Account($mailAccount);
225+
$localMessage = new LocalMessage();
226+
$localMessage->setSubject('Test');
227+
$localMessage->setBodyPlain('Test');
228+
$localMessage->setHtml(false);
229+
$localMessage->setAliasId(1);
230+
$localMessage->setRequestMdn(true);
231+
$transport = $this->createMock(Horde_Mail_Transport::class);
232+
$this->smtpClientFactory->expects($this->once())
233+
->method('create')
234+
->willReturn($transport);
235+
$this->aliasService->expects(self::once())
236+
->method('find')
237+
->willThrowException(new DoesNotExistException('Alias does not exist'));
238+
$this->transmissionService->expects(self::once())
239+
->method('getSignMimePart')
240+
->willReturnCallback(static fn ($localMessage, $account, $mimePart) => $mimePart);
241+
$this->transmissionService->expects(self::once())
242+
->method('getEncryptMimePart')
243+
->willReturnCallback(static fn ($localMessage, $to, $cc, $bcc, $account, $mimePart) => $mimePart);
244+
245+
// Act
172246
$this->transmission->sendMessage($account, $localMessage);
247+
248+
// Assert
173249
$this->assertEquals(LocalMessage::STATUS_RAW, $localMessage->getStatus());
250+
$this->assertStringContainsString('From: Bob <bob@example.org', $localMessage->getRaw());
251+
$this->assertStringContainsString('Disposition-Notification-To: Bob <bob@example.org>', $localMessage->getRaw());
174252
}
175253

176254
public function testSendNewMessageWithMessageAsAttachment() {

0 commit comments

Comments
 (0)